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

display new bulk action statuses in UI #24409

Conversation

jamiedemaria
Copy link
Contributor

@jamiedemaria jamiedemaria commented Sep 11, 2024

Summary & Motivation

Updates the FE to display the COMPLETED_SUCCESS and COMPLETED_FAILED statuses in the downstream PR

How I Tested These Changes

Backfill run after updating the UI shows Succeeded status. Backfill run before the UI update is still Completed. Note that we changed the language to Success instead of Succeeded so this screenshot is slightly out of date

Screenshot 2024-09-12 at 12 40 50 PM

Changelog

Insert changelog entry or "NOCHANGELOG" here.

  • NEW (added new feature or capability)
  • BUGFIX (fixed a bug)
  • DOCS (added or updated documentation)

Copy link
Contributor Author

jamiedemaria commented Sep 11, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @jamiedemaria and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Sep 11, 2024

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-641q0ts0b-elementl.vercel.app
https://jamie-bulk-action-status-fe.core-storybook.dagster-docs.io

Built with commit fe62f0f.
This pull request is being automatically deployed with vercel-action

@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from dd90824 to 6ccf131 Compare September 11, 2024 19:09
@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from c0f3355 to ea8ea41 Compare September 11, 2024 19:10
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 6ccf131 to 55bc124 Compare September 11, 2024 19:10
@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from ea8ea41 to 2a8e135 Compare September 11, 2024 19:33
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 55bc124 to cf3cda5 Compare September 11, 2024 19:33
@jamiedemaria jamiedemaria marked this pull request as ready for review September 11, 2024 20:53
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from cf3cda5 to 053a692 Compare September 11, 2024 21:01
@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from 2a8e135 to 0923a4b Compare September 12, 2024 16:19
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 053a692 to 836261e Compare September 12, 2024 16:19
@@ -42,6 +42,10 @@ const labelForBackfillStatus = (key: BulkActionStatus) => {
return 'Failed';
case BulkActionStatus.REQUESTED:
return 'In progress';
case BulkActionStatus.COMPLETED_SUCCESS:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I matched the tense of the existing labels for this PR, but the runs page uses success and failure instead of succeeded and failed. should i align them?

cc @salazarm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe not so important for this PR, but we should just keep it in mind to make sure the runs feed uses consistent terminology

Copy link
Contributor

Choose a reason for hiding this comment

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

The tenses within DagsterRunStatus and step events are also annoyingly inconsistent with each other. E.g. STARTED vs. FAILURE. However I think we use SUCCESS pretty consistently throughout the product, so I support using that instead of SUCCEEDED.

@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from 0923a4b to f097359 Compare September 12, 2024 19:44
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 836261e to fff1c4e Compare September 12, 2024 19:44
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from fff1c4e to 3e5d992 Compare September 13, 2024 11:28
@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from 924c8a3 to 8322a44 Compare September 13, 2024 11:54
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 3e5d992 to 5bd30fe Compare September 13, 2024 11:54
@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from 8322a44 to 8f9343e Compare September 13, 2024 15:36
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 5bd30fe to 617f667 Compare September 13, 2024 15:37
@jamiedemaria jamiedemaria force-pushed the jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and branch from 8f9343e to 7b447bb Compare September 16, 2024 14:42
@jamiedemaria jamiedemaria force-pushed the jamie/bulk-action-status-fe branch from 617f667 to fe62f0f Compare September 16, 2024 14:43
@jamiedemaria jamiedemaria merged commit 26d0664 into jamie/fou-385-expand-bulkactionstatus-to-encompass-completed-success-and Sep 16, 2024
1 of 2 checks passed
@jamiedemaria jamiedemaria deleted the jamie/bulk-action-status-fe branch September 16, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants