Skip to content

Commit

Permalink
[ui] Update DA evaluation page to show backfill in table if a backfil…
Browse files Browse the repository at this point in the history
…l was requested (#26643)

## Summary & Motivation
The runs table for a DA evaluation was not showing backfills when DA
requested a backfill
<img width="1440" alt="Screenshot 2024-12-20 at 10 55 57 AM"
src="https://github.com/user-attachments/assets/e38e5598-4e35-4219-a0de-9228b99d595e"
/>

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.


<img width="1440" alt="Screenshot 2024-12-20 at 11 18 30 AM"
src="https://github.com/user-attachments/assets/2541bb18-b011-49d9-b271-56f5f022cf64"
/>

I also had to update the `RunsFeedTableWithFilters` component to take a
bool `includeRunsFromBackfills` 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 of `RunsFeedTableWithFilters` to pass the bool

## How 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.
  • Loading branch information
jamiedemaria authored Dec 27, 2024
1 parent 8c4f449 commit 37f2042
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,21 @@ export const AutomaterializeMiddlePanelWithData = ({

const {partitions: allPartitions} = usePartitionsForAssetKey(definition?.assetKey.path || []);

// For a single asset for a single tick, DA will either request a list of runs or a single backfill.
// When DA requests a backfill we want to show a row for that backfill in the table, so we need to
// construct the RunsFilter differently. Backfill IDs are 8 characters long, so we can use that to
// determine if a backfill was requested. If DA is updated to request multiple backfills in a
// single evaluation, or emit a combination of runs and backfills in a single evaluation, this
// logic will need to be updated.
const backfillIdLength = 8;
const runsFilter: RunsFilter | null = useMemo(
() => (selectedEvaluation?.runIds.length ? {runIds: selectedEvaluation.runIds} : null),
() =>
selectedEvaluation?.runIds.length
? selectedEvaluation.runIds.length === 1 &&
selectedEvaluation.runIds[0]?.length === backfillIdLength
? {tags: [{key: 'dagster/backfill', value: selectedEvaluation.runIds[0]}]}
: {runIds: selectedEvaluation.runIds}
: null,
[selectedEvaluation],
);

Expand Down Expand Up @@ -163,7 +176,7 @@ export const AutomaterializeMiddlePanelWithData = ({
{flagLegacyRunsPage ? (
<AutomaterializeRunsTable runIds={selectedEvaluation.runIds} />
) : runsFilter ? (
<RunsFeedTableWithFilters filter={runsFilter} />
<RunsFeedTableWithFilters filter={runsFilter} includeRunsFromBackfills={false} />
) : (
<Box padding={{vertical: 12}}>
<NonIdealState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const EvaluationRunInfo = ({runIds, timestamp}: EvaluationRunInfoProps) => {
}
/>
<div style={{flex: 1, overflowY: 'auto'}}>
<RunsFeedTableWithFilters filter={{runIds}} />
<RunsFeedTableWithFilters filter={{runIds}} includeRunsFromBackfills={true} />
</div>
<DialogFooter topBorder>
<Button onClick={() => setIsOpen(false)}>Done</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ export const GlobalAutomaterializationContent = () => {
<RunsFeedTableWithFilters
filter={RUNS_FILTER}
actionBarComponents={tableViewSwitch}
includeRunsFromBackfills={true}
/>
</Box>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ export const BackfillRunsTab = ({
belowActionBarComponents={belowActionBarComponents}
hideTags={BACKFILL_TAGS}
scroll={true}
includeRunsFromBackfills={true}
emptyState={() => (
<Box
padding={{vertical: 24}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,20 @@ export const RunsFeedTable = ({
export const RunsFeedTableWithFilters = ({
filter,
scroll,
includeRunsFromBackfills,
...rest
}: {
filter: RunsFilter;
includeRunsFromBackfills: boolean;
} & Pick<
RunsFeedTableProps,
'actionBarComponents' | 'belowActionBarComponents' | 'emptyState' | 'hideTags' | 'scroll'
>) => {
const {entries, paginationProps, queryResult} = useRunsFeedEntries(filter, 'all', true);
const {entries, paginationProps, queryResult} = useRunsFeedEntries(
filter,
'all',
includeRunsFromBackfills,
);
const refreshState = useQueryRefreshAtInterval(queryResult, FIFTEEN_SECONDS);

function content() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ export const ScheduleRoot = (props: Props) => {
tabs={tabs}
/>
) : (
<RunsFeedTableWithFilters filter={runsFilter} actionBarComponents={tabs} />
<RunsFeedTableWithFilters
filter={runsFilter}
actionBarComponents={tabs}
includeRunsFromBackfills={true}
/>
)}
</Page>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ export const SensorPageAutomaterialize = (props: Props) => {
<RunsFeedTableWithFilters
filter={runTableFilter}
actionBarComponents={tableViewSwitch}
includeRunsFromBackfills={true}
/>
)}
</Box>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,9 @@ def build_run_requests(
run_tags: Optional[Mapping[str, str]],
emit_backfills: bool,
) -> Sequence[RunRequest]:
"""For a single asset in a given tick, the asset will only be part of a run or a backfill, not both.
If the asset is targetd by a backfill, there will only be one backfill that targets the asset.
"""
if emit_backfills:
backfill_run_request, entity_subsets = _build_backfill_request(
entity_subsets, asset_graph, run_tags
Expand Down

1 comment on commit 37f2042

@github-actions
Copy link

Choose a reason for hiding this comment

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

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-mqhc79msx-elementl.vercel.app

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

Please sign in to comment.