Add gantt chart view of job peers #153
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jpwhite4
force-pushed
the
peers
branch
4 times, most recently
from
fba0055
to
a787fb1
Compare
jpwhite4
changed the title
plessbd
approved these changes
Jun 15, 2017
ryanrath
reviewed
Jun 16, 2017
There was a problem hiding this comment.
Overall looks good to me. I did run into a few things when testing it out on my dev port though:
- when hovering over the y axis labels the mouse changes to the 'finger' like you should be able to click on them but nothing happens when you do.
- Hovering over the peer bars creates no change in the mouse pointer but clicking on the bar does take you to a 'Peers of search w/ that job selected.
- Clicking the 'Current' bar raises an error in GanntChart.js:43 due to not having a evt.point.ref value. Don't know if this is meant to be clickable or not but thought I'd let you know what I found.
So yeah, other than those few things it looks good.
| @@ -1352,6 +1360,72 @@ public function _processJobSearchByAction(Request $request, Application $app, XD | |||
| return $results; | |||
| } | |||
|
|
|||
| protected function getJobPeers(Application $app, XDUser $user, $realm, $jobId, $start, $limit) | |||
There was a problem hiding this comment.
Documentation for the function wouldn't suck
- Added new REST endpoint to retrieve the peer data - Added new gantt chart tab
- Added docstring to getJobPeers function - Click events only added to job peers not the job itself. - Cursor changed to a pointer when the click event is available.
tyearke
added
the
enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Change the Peers node in the search history so that it opens a new tab with an interactive graphical display of the job's peer jobs (i.e. the jobs that ran on the same shared hardware). The individual job peers no longer show as leaf nodes because this display was not useful and did not scale when there were lots of job peers. Jobs with many peers are now handled using the paging toolbar in the graphical display.
These code changes:
Note that the new ChartTab base class is intended to be used as the base class for all job viewer timeseries plots. This should significantly reduce the amount of cut-and-paste boilerplate code. A future pull request is planned to switch all charts over to this code.
The job peers click uses the existing mechanism that supports the click through from the metric explorer.
Motivation and Context
Improve ability to analyse shared jobs.
Tests performed
Installed on metrics-dev manually tested. Tests included:
Types of changes
Checklist: