-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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] Update DA evaluation page to show backfill in table if a backfill was requested #26643
base: master
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit aabed95. |
() => (selectedEvaluation?.runIds.length ? {runIds: selectedEvaluation.runIds} : null), | ||
() => | ||
selectedEvaluation?.runIds.length | ||
? selectedEvaluation.runIds.length === 1 && selectedEvaluation.runIds[0]?.length === 8 |
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.
why does this length need to be 8
?
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.
That's the length of a backfill 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.
ooooooo got it
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.
So wait, can't there be multiple runIds ?
f2373ce
to
997a5eb
Compare
cbaa1ab
to
aabed95
Compare
Summary & Motivation
The runs table for a DA evaluation was not showing backfills when DA requested a backfill
This is because the the
selectedEvaluation.runIds
could be run ids or a backfill id, but we were always adding them as run id filters. Then the value was a backfill id, it would return no results.This add some (admittedly a bit hacky) logic to determine if the id is a backfill id, and set the correct filter if so. A more long term solution would be to store run ids and backfill ids as separate lists on the evaluation, but this will fix the problem until we can make the more sustainable change.
I also had to update the
RunsFeedTableWithFilters
component to take a boolincludeRunsFromBackfills
so that we would only show the backfill row, not the runs that the backfill launched. I had to then update all of the users ofRunsFeedTableWithFilters
to pass the boolHow I Tested These Changes
👀
Changelog
[ui] Fixed a bug where backfills launched by Declarative Automation were not being shown in the table of launched runs