Skip to content
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] Multi-condition start/revert/edit buttons when a job isn't running #24985

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

philrenaud
Copy link
Contributor

Stopped:
image

Failed, but once upon a time it was running:
image

Failed, has always been a failure:
image

Resolves #24980

@philrenaud philrenaud self-assigned this Jan 31, 2025
@philrenaud philrenaud linked an issue Jan 31, 2025 that may be closed by this pull request
@@ -1215,3 +1217,117 @@ function getScenarioQueryParameter() {
return mirageScenario;
}
/* eslint-enable */

export function createRestartableJobs(server) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sets jobs/versions up both in our mocked mirage smallCluster (where we can preview this), but also in a way that our tests can leverage it easily via import.

@philrenaud philrenaud force-pushed the f-ui/24980-better-job-start-button branch from 02e2161 to cad7a7a Compare January 31, 2025 19:03
@philrenaud philrenaud marked this pull request as ready for review January 31, 2025 19:18
@philrenaud philrenaud requested review from a team as code owners January 31, 2025 19:18
tgross
tgross previously approved these changes Jan 31, 2025
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines 34 to 35
// TODO: looks like this watcher is pushing me over the "Browser is freaking out" max connections limit
// versions: this.watchVersions.perform(model),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does commenting this out leave the watchRelationship defined below orphaned? Any consequence to that? Missing this doesn't seem to generate any extra HTTP requests from what I can see in the devtools.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it's only this line in the controller.set('watchers' bit that causes the extra blocking-query stuff.

I left this commented out because I wanted to be consistent on this page about how we watch for resource updates, but I think I've just made peace with loading it on-demand via the component. Going to remove the versions-loading stuff from the route file.

tgross
tgross previously approved these changes Jan 31, 2025
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

arodd
arodd previously approved these changes Feb 3, 2025
Copy link

@arodd arodd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@philrenaud philrenaud added the backport/1.9.x backport to 1.9.x release line label Feb 3, 2025
@philrenaud philrenaud dismissed stale reviews from arodd and tgross via aa3eba3 February 3, 2025 20:17
@philrenaud philrenaud force-pushed the f-ui/24980-better-job-start-button branch from b146684 to aa3eba3 Compare February 3, 2025 20:17
@philrenaud philrenaud requested a review from arodd February 3, 2025 20:17
@philrenaud philrenaud merged commit 389f461 into main Feb 4, 2025
22 checks passed
@philrenaud philrenaud deleted the f-ui/24980-better-job-start-button branch February 4, 2025 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.9.x backport to 1.9.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui] Improve the functionality of "Start Job" in the UI
3 participants