[ui] update backfill cancel button to only have one option #27272
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
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 44c0cf6. |
jamiedemaria
changed the title
jamiedemaria
commented
Jan 22, 2025
| ((backfill.isAssetBackfill && backfill.status === BulkActionStatus.REQUESTED) || | ||
| backfill.numCancelable > 0) | ||
| ); | ||
| return backfill.hasCancelPermission && backfill.status === BulkActionStatus.REQUESTED; |
There was a problem hiding this comment.
since job backfills can be canceled via the daemon now, we don't need this additional check. I think this is vestigial from a time when job-backfills were canceled by terminating runs that were in-progress
salazarm
approved these changes
Jan 22, 2025
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.
Summary & Motivation
Previously the backfill cancellation button had two titles "Cancel backfill submission" and "Terminate unfinished runs". Over time it seems that most of the cases when "Terminate unfinished runs" was shown got phased out, and "Cancel backfill submission" is a misnomer because it doesn't stop the backfill from being added to the DB, it cancels the backfill and all runs it has launched.
Additionally, in the remaining case when "Terminate unfinished runs" was shown, it used a different code path to just cancel the runs launched by the backfill and didn't cancel the backfill itself, potentially leading to a confusing state where a user has "canceled" the backfill but the backfill is still running and submitting new runs.
This PR consolidates the button so that it just has one behavior: sending a cancellation query to the backfill. This will signal to the daemon to cancel any in-progress runs, then mark the backfill as canceled. Additionally it allows us to immediately exit the dialog that appears when a user cancels a backfill. The button is also renamed to make its action more clear
Screen recording of canceling a backfill
https://github.com/user-attachments/assets/345a27b9-aa07-4aeb-9960-c6524251565e
If a user doens't have permission to cancel a backfill or the backfill cannot be canceled (already completed), the button is greyed out
How I Tested These Changes
Changelog