-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ui] Handle parent/child jobs with the paginated Jobs Index route #20493
[ui] Handle parent/child jobs with the paginated Jobs Index route #20493
Conversation
Ember Test Audit comparison
|
1afa335
to
09fa243
Compare
@@ -0,0 +1,49 @@ | |||
{{! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this is a copy/paste of the existing component, but because we have sufficient deviation in what we show here, it made more sense for it to be its own component rather than deal with conditionals.
get children() { | ||
return this.get('job.children') || []; | ||
return this.jobs || []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A big part of this PR is going from "Derive child jobs from the parent job's model's computed properties", to "accept the child jobs as passed in directly"
{{@completeAllocs}}/{{@groupCountSum}} | ||
{{else}} | ||
{{@runningAllocs}}/{{@groupCountSum}} | ||
{{/if}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Felt it made more sense, in the context of a list of parameterized/periodic child jobs, which are going to be batch/sysbatch, to tally up the completed allocations rather than the running ones.
This is inconsistent with what we show on the jobs index page (running/expected), but contextually probably more relevant. I'm only feeling about 75% confident of this change so open to undoing it.
break; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally follows the main "duelling queries" pattern from the jobs index list of having the route's setupController
kick off a restartable ember concurrency task, rather than use @watchQuery
, which has some "when do I invalidate the cache" issues.
controller.resetQueryIndex({ | ||
id: model.get('plainId'), | ||
namespace: model.get('namespace.id'), | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels extraneous, but otherwise loading a parent job, then back out, then another parent job, then back out, then the first one again results in the child jobs from the second job being present due to the blocking query run at page load still having the previous-version's index.
There is probably another way to do this, but this appears to be working as expected.
09fa243
to
b5a6a15
Compare
b5a6a15
to
046750e
Compare
cf94d52
to
7532eb7
Compare
046750e
to
0f70fa3
Compare
baa42fe
to
3350ebc
Compare
5aa18c8
to
9a8993c
Compare
… issue where moving from parent to parent would not reset index
9a8993c
to
c6a34df
Compare
c6a34df
to
d3ced52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…0493) * First pass at a non-watchQuery version * Parameterized jobs get child fetching and jobs index status style for parent jobs * Completed allocs vs Running allocs in a child-job context, and fix an issue where moving from parent to parent would not reset index * Testfix and better handling empty-child-statuses-list * Parent/child test case * Dont show empty allocation-status bars for parent jobs with no children
* Hook and latch on the initial index * Serialization and restart of controller and table * de-log * allocBlocks reimplemented at job model level * totalAllocs doesnt mean on jobmodel what it did in steady.js * Hamburgers to sausages * Hacky way to bring new jobs back around and parent job handling in list view * Getting closer to hook/latch * Latch from update on hook from initialize, but fickle * Note on multiple-watch problem * Sensible monday morning comment removal * use of abortController to handle transition and reset events * Next token will now update when there's an on-page shift * Very rough anti-jostle technique * Demoable, now to move things out of route and into controller * Into the controller, generally * Smarter cancellations * Reset abortController on index models run, and system/sysbatch jobs now have an improved groupCountSum computed property * Prev Page reverse querying * n+1th jobs existing will trigger nextToken/pagination display * Start of a GET/POST statuses return * Namespace fix * Unblock tests * Realizing to my small horror that this skipURLModification flag may be too heavy handed * Lintfix * Default liveupdates localStorage setting to true * Pagination and index rethink * Big uncoupling of watchable and url-append stuff * Testfixes for region, search, and keyboard * Job row class for test purposes * Allocations in test now contain events * Starting on the jobs list tests in earnest * Forbidden state de-bubbling cleanup * Job list page size fixes * Facet/Search/Filter jobs list tests skipped * Maybe it's the automatic mirage logging * Unbreak task unit test * Pre-sort sort * styling for jobs list pagination and general PR cleanup * moving from Job.ActiveDeploymentID to Job.LatestDeployment.ID * modifyIndex-based pagination (#20350) * modifyIndex-based pagination * modifyIndex gets its own column and pagination compacted with icons * A generic withPagination handler for mirage * Some live-PR changes * Pagination and button disabled tests * Job update handling tests for jobs index * assertion timeout in case of long setTimeouts * assert.timeouts down to 500ms * de-to-do * Clarifying comment and test descriptions * Bugfix: resizing your browser on the new jobs index page would make the viz grow forever (#20458) * [ui] Searching and filtering options (#20459) * Beginnings of a search box for filter expressions * jobSearchBox integration test * jobs list updateFilter initial test * Basic jobs list filtering tests * First attempt at side-by-side facets and search with a computed filter * Weirdly close to an iterative approach but checked isnt tracked properly * Big rework to make filter composition and decomposition work nicely with the url * Namespace facet dropdown added * NodePool facet dropdown added * hdsFacet for future testing and basic namespace filtering test * Namespace filter existence test * Status filtering * Node pool/dynamic facet test * Test patchups * Attempt at optimize test fix * Allocation re-load on optimize page explainer * The Big Un-Skip * Post-PR-review cleanup * todo-squashing * [ui] Handle parent/child jobs with the paginated Jobs Index route (#20493) * First pass at a non-watchQuery version * Parameterized jobs get child fetching and jobs index status style for parent jobs * Completed allocs vs Running allocs in a child-job context, and fix an issue where moving from parent to parent would not reset index * Testfix and better handling empty-child-statuses-list * Parent/child test case * Dont show empty allocation-status bars for parent jobs with no children * Splits Settings into 2 sections, sign-in/profile and user settings (#20535) * Changelog
Brings child jobs to the /statuses endpoint when a flag is present, and solves the issue of "Parent jobs presume knowledge of the entirety of a cluster's jobs" + "the index page is now paginated" from this project at large.
This also makes some quality of life updates to the existing child jobs table on a job page, and makes monitoring parent jobs from the main/index page more useful (with status badges that react to live updates and tallies of child jobs)
Resolves #20491