Skip to content

Commit

Permalink
[ui] update backfill cancel button to only have one option (#27272)
Browse files Browse the repository at this point in the history
## 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
<img width="1495" alt="Screenshot 2025-01-22 at 12 02 39 PM"
src="https://github.com/user-attachments/assets/92219355-1264-47dd-91f4-a7fe48472722"
/>


## How I Tested These Changes

## Changelog

> Insert changelog entry or delete this section.
  • Loading branch information
jamiedemaria authored and prha committed Jan 29, 2025
1 parent 14f0c27 commit d3418ef
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,11 @@ import {getBackfillPath} from '../../runs/RunsFeedUtils';
import {runsPathWithFilters} from '../../runs/RunsFilterInput';
import {AnchorButton} from '../../ui/AnchorButton';

export function backfillCanCancelSubmission(backfill: {
export function backfillCanCancel(backfill: {
hasCancelPermission: boolean;
isAssetBackfill: boolean;
status: BulkActionStatus;
numCancelable: number;
}) {
return (
backfill.hasCancelPermission &&
((backfill.isAssetBackfill && backfill.status === BulkActionStatus.REQUESTED) ||
backfill.numCancelable > 0)
);
return backfill.hasCancelPermission && backfill.status === BulkActionStatus.REQUESTED;
}

export function backfillCanResume(backfill: {
Expand All @@ -49,24 +43,12 @@ export function backfillCanResume(backfill: {
);
}

export function backfillCanCancelRuns(
backfill: {hasCancelPermission: boolean},
hasCancelableRuns: boolean,
) {
if (!backfill.hasCancelPermission || !hasCancelableRuns) {
return false;
}
return hasCancelableRuns;
}

export const BackfillActionsMenu = ({
backfill,
canCancelRuns,
refetch,
anchorLabel,
}: {
backfill: BackfillActionsBackfillFragment;
canCancelRuns: boolean;
refetch: () => void;
anchorLabel?: string;
}) => {
Expand Down Expand Up @@ -117,7 +99,7 @@ export const BackfillActionsMenu = ({
}
};

const canCancelSubmission = backfillCanCancelSubmission(backfill);
const canCancel = backfillCanCancel(backfill);

const popover = (
<Popover
Expand Down Expand Up @@ -145,10 +127,10 @@ export const BackfillActionsMenu = ({
onClick={() => resume()}
/>
<MenuItem
text={canCancelSubmission ? 'Cancel backfill submission' : 'Terminate unfinished runs'}
text="Cancel backfill"
icon="cancel"
intent="danger"
disabled={!(canCancelSubmission || canCancelRuns)}
disabled={!canCancel}
onClick={() => setShowTerminateDialog(true)}
/>
</Menu>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,7 @@ export const BackfillPage = () => {
<Box flex={{gap: 12, alignItems: 'center'}}>
{isInProgress ? <QueryRefreshCountdown refreshState={refreshState} /> : null}
{backfill ? (
<BackfillActionsMenu
backfill={backfill}
refetch={queryResult.refetch}
canCancelRuns={backfill.status === BulkActionStatus.REQUESTED}
/>
<BackfillActionsMenu backfill={backfill} refetch={queryResult.refetch} />
) : null}
</Box>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react';
import {Link} from 'react-router-dom';
import styled from 'styled-components';

import {BackfillActionsMenu, backfillCanCancelRuns} from './BackfillActionsMenu';
import {BackfillActionsMenu} from './BackfillActionsMenu';
import {BackfillStatusTagForPage} from './BackfillStatusTagForPage';
import {SingleBackfillQuery, SingleBackfillQueryVariables} from './types/BackfillRow.types';
import {BackfillTableFragment} from './types/BackfillTable.types';
Expand Down Expand Up @@ -38,7 +38,7 @@ export const BackfillRow = (props: BackfillRowProps) => {
props.backfill.isAssetBackfill;

if (statusUnsupported) {
return <BackfillRowContent {...props} hasCancelableRuns={false} statusQueryResult={null} />;
return <BackfillRowContent {...props} statusQueryResult={null} />;
}
return (
<BackfillRowLoader backfillId={props.backfill.id}>
Expand All @@ -48,7 +48,6 @@ export const BackfillRow = (props: BackfillRowProps) => {
};

interface LoadResult {
hasCancelableRuns: boolean;
statusQueryResult: QueryResult<any, any> | null;
}

Expand All @@ -72,15 +71,7 @@ export const BackfillRowLoader = (props: {

useQueryRefreshAtInterval(statusQueryResult, FIFTEEN_SECONDS);

const {data} = statusQueryResult;
const {hasCancelableRuns} = React.useMemo(() => {
if (data?.partitionBackfillOrError.__typename === 'PartitionBackfill') {
return {hasCancelableRuns: data.partitionBackfillOrError.cancelableRuns.length > 0};
}
return {hasCancelableRuns: false};
}, [data]);

return props.children({hasCancelableRuns, statusQueryResult});
return props.children({statusQueryResult});
};

export const BackfillRowContent = ({
Expand All @@ -89,7 +80,6 @@ export const BackfillRowContent = ({
showBackfillTarget,
onShowPartitionsRequested,
refetch,
hasCancelableRuns,
statusQueryResult,
}: BackfillRowProps & LoadResult) => {
const repoAddress = backfill.partitionSet
Expand Down Expand Up @@ -133,11 +123,7 @@ export const BackfillRowContent = ({
</td>
<td style={{width: 140}}>{renderBackfillStatus()}</td>
<td>
<BackfillActionsMenu
backfill={backfill}
canCancelRuns={backfillCanCancelRuns(backfill, hasCancelableRuns)}
refetch={refetch}
/>
<BackfillActionsMenu backfill={backfill} refetch={refetch} />
</td>
</tr>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import {Button, Dialog, DialogBody, DialogFooter} from '@dagster-io/ui-components';
import {useMemo, useState} from 'react';
import {useState} from 'react';

import {SINGLE_BACKFILL_CANCELABLE_RUNS_QUERY} from './BackfillRow';
import {BackfillTerminationDialogBackfillFragment} from './types/BackfillFragments.types';
import {SingleBackfillQuery, SingleBackfillQueryVariables} from './types/BackfillRow.types';
import {
CancelBackfillMutation,
CancelBackfillMutationVariables,
} from './types/BackfillTerminationDialog.types';
import {gql, useMutation, useQuery} from '../../apollo-client';
import {gql, useMutation} from '../../apollo-client';
import {PYTHON_ERROR_FRAGMENT} from '../../app/PythonErrorFragment';
import {BulkActionStatus} from '../../graphql/types';
import {TerminationDialog} from '../../runs/TerminationDialog';

interface Props {
backfill?: BackfillTerminationDialogBackfillFragment;
Expand All @@ -23,34 +20,8 @@ export const BackfillTerminationDialog = ({backfill, onClose, onComplete}: Props
const [cancelBackfill] = useMutation<CancelBackfillMutation, CancelBackfillMutationVariables>(
CANCEL_BACKFILL_MUTATION,
);
const {data} = useQuery<SingleBackfillQuery, SingleBackfillQueryVariables>(
SINGLE_BACKFILL_CANCELABLE_RUNS_QUERY,
{
variables: {
backfillId: backfill?.id || '',
},
notifyOnNetworkStatusChange: true,
skip: !backfill,
},
);
const [isSubmitting, setIsSubmitting] = useState(false);
const unfinishedMap = useMemo(() => {
if (!backfill || !data || data.partitionBackfillOrError.__typename !== 'PartitionBackfill') {
return {};
}
return (
data.partitionBackfillOrError.cancelableRuns?.reduce(
(accum, run) => {
if (run && run.runId) {
accum[run.runId] = true;
}
return accum;
},
{} as Record<string, boolean>,
) || {}
);
}, [backfill, data]);
if (!backfill || !data) {
if (!backfill) {
return null;
}

Expand Down Expand Up @@ -99,18 +70,6 @@ export const BackfillTerminationDialog = ({backfill, onClose, onComplete}: Props
)}
</DialogFooter>
</Dialog>
{!backfill.isAssetBackfill && unfinishedMap && (
<TerminationDialog
isOpen={
!!backfill &&
(!numUnscheduled || backfill.status !== 'REQUESTED') &&
!!Object.keys(unfinishedMap).length
}
onClose={onClose}
onComplete={onComplete}
selectedRuns={unfinishedMap}
/>
)}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,7 @@ export const RunsFeedBackfillPage = () => {
<Box flex={{gap: 12, alignItems: 'center'}}>
{isInProgress ? <QueryRefreshCountdown refreshState={refreshState} /> : null}
{backfill ? (
<BackfillActionsMenu
backfill={backfill}
refetch={queryResult.refetch}
canCancelRuns={backfill.status === BulkActionStatus.REQUESTED}
/>
<BackfillActionsMenu backfill={backfill} refetch={queryResult.refetch} />
) : null}
</Box>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {RunFilterToken} from './RunsFilterInput';
import {RunTimeFragment} from './types/RunUtils.types';
import {RunsFeedTableEntryFragment} from './types/RunsFeedTableEntryFragment.types';
import {RunStatus} from '../graphql/types';
import {BackfillActionsMenu, backfillCanCancelRuns} from '../instance/backfill/BackfillActionsMenu';
import {BackfillActionsMenu} from '../instance/backfill/BackfillActionsMenu';
import {BackfillTarget} from '../instance/backfill/BackfillRow';
import {HeaderCell, HeaderRow, RowCell} from '../ui/VirtualizedTable';
import {appendCurrentQueryParams} from '../util/appendCurrentQueryParams';
Expand Down Expand Up @@ -195,7 +195,6 @@ export const RunsFeedRow = ({
{entry.__typename === 'PartitionBackfill' ? (
<BackfillActionsMenu
backfill={{...entry, status: entry.backfillStatus}}
canCancelRuns={backfillCanCancelRuns(entry, entry.numCancelable > 0)}
refetch={refetch}
anchorLabel="View"
/>
Expand Down

0 comments on commit d3418ef

Please sign in to comment.