Skip to content

Commit

Permalink
[ui] Leverage ReexecutionStrategy for all “Re-execute” buttons except…
Browse files Browse the repository at this point in the history
… step subset (#16731)

## Summary & Motivation

This is a follow-up to #16728.

I noticed this afternoon that we have some code paths in the UI which
re-execute runs by passing a `ReexecutionStrategy` to the GraphQL
mutation, and older code paths which generate the entire re-execution of
the run from the base data (config YAML etc etc) themselves and pass it
as execution metadata.

Ironically the bug earlier today was that the latter was working and the
former was not. To increase the chances that we catch bugs we want all
re-executions to use the same machinery regardless of how they're
started in the UI.

This PR updates the Run Table and Run Details page to use
ReexecutionStrategy for all callsites except two -- the "Selected Step"
and "From Selected Step" options here. These two still assemble run
execution variables manually.

<img width="682" alt="image"
src="https://github.com/dagster-io/dagster/assets/1037212/ecc2f2c3-c428-484b-8ef8-b0b4cfd98e83">

Nits:

This PR also fixes two tiny issues:

- The per-run actions dropdown goes from "Loading configuration" to
"View configuration" which changes the width of the dropdown about 1s
after it appears. This looks bad so I put a min-width on the menu item

- The Actions button on the Runs Table was indented too much on the
right side

<img width="396" alt="image"
src="https://github.com/dagster-io/dagster/assets/1037212/5847322b-16c1-4df5-b615-1b48ad196607">


## How I Tested These Changes

I re-executed a variety of runs from both the Runs table actions menu,
Runs table per-run dropdown menu, and Runs detail page.

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
bengotow and bengotow authored Sep 25, 2023
1 parent 4a1e72c commit 47ad505
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import {Button, Icon, Menu, MenuItem, Popover, Tooltip} from '@dagster-io/ui-com
import * as React from 'react';

import {usePermissionsForLocation} from '../app/Permissions';
import {ReexecutionStrategy} from '../graphql/types';
import {canRunAllSteps, canRunFromFailure} from '../runs/RunActionButtons';
import {RUN_FRAGMENT} from '../runs/RunFragments';
import {RunTimeFragment} from '../runs/types/RunUtils.types';
import {useJobReExecution} from '../runs/useJobReExecution';
import {useJobReexecution} from '../runs/useJobReExecution';
import {MenuLink} from '../ui/MenuLink';
import {RepoAddress} from '../workspace/types';
import {workspacePipelinePath} from '../workspace/workspacePath';
Expand Down Expand Up @@ -43,13 +44,13 @@ export const JobMenu = (props: Props) => {
}
}, [lastRun, fetchHasExecutionPlan]);

const onLaunch = useJobReExecution(run);
const onReexecute = useJobReexecution();

const reExecuteAllItem = (
<MenuItem
icon="replay"
text="Re-execute latest run"
onClick={() => onLaunch({type: 'all'})}
onClick={() => (run ? onReexecute(run, ReexecutionStrategy.ALL_STEPS) : undefined)}
disabled={!canLaunchPipelineReexecution || !run || !canRunAllSteps(run)}
/>
);
Expand All @@ -58,7 +59,7 @@ export const JobMenu = (props: Props) => {
<MenuItem
icon="sync_problem"
text="Re-execute latest run from failure"
onClick={() => onLaunch({type: 'from-failure'})}
onClick={() => (run ? onReexecute(run, ReexecutionStrategy.FROM_FAILURE) : undefined)}
disabled={!canLaunchPipelineReexecution || !run || !canRunFromFailure(run)}
/>
);
Expand Down
4 changes: 0 additions & 4 deletions js_modules/dagster-ui/packages/ui-core/src/runs/Run.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
useComputeLogFileKeyForSelection,
matchingComputeLogKeyFromStepKey,
} from './useComputeLogFileKeyForSelection';
import {useJobReExecution} from './useJobReExecution';
import {useQueryPersistedLogFilter} from './useQueryPersistedLogFilter';

interface RunProps {
Expand Down Expand Up @@ -167,8 +166,6 @@ const RunWithData: React.FC<RunWithDataProps> = ({
onSetLogsFilter,
onSetSelectionQuery,
}) => {
const onLaunch = useJobReExecution(run);

const [queryLogType, setQueryLogType] = useQueryPersistedState<string>({
queryKey: 'logType',
defaults: {logType: LogType.structured},
Expand Down Expand Up @@ -298,7 +295,6 @@ const RunWithData: React.FC<RunWithDataProps> = ({
</Tooltip>
<RunActionButtons
run={run}
onLaunch={onLaunch}
graph={runtimeGraph}
metadata={metadata}
selection={{query: selectionQuery, keys: selectionStepKeys}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,25 @@ import * as React from 'react';
import {showSharedToaster} from '../app/DomUtils';
import {filterByQuery, GraphQueryItem} from '../app/GraphQueryImpl';
import {DEFAULT_DISABLED_REASON} from '../app/Permissions';
import {ReexecutionStrategy} from '../graphql/types';
import {LaunchButtonConfiguration, LaunchButtonDropdown} from '../launchpad/LaunchButton';
import {useRepositoryForRunWithParentSnapshot} from '../workspace/useRepositoryForRun';

import {IRunMetadataDict, IStepState} from './RunMetadataProvider';
import {doneStatuses, failedStatuses} from './RunStatuses';
import {DagsterTag} from './RunTag';
import {ReExecutionStyle} from './RunUtils';
import {getReexecutionParamsForSelection} from './RunUtils';
import {StepSelection} from './StepSelection';
import {TerminationDialog, TerminationState} from './TerminationDialog';
import {RunFragment, RunPageFragment} from './types/RunFragments.types';
import {useJobAvailabilityErrorForRun} from './useJobAvailabilityErrorForRun';
import {useJobReexecution} from './useJobReExecution';

interface RunActionButtonsProps {
run: RunPageFragment;
selection: StepSelection;
graph: GraphQueryItem[];
metadata: IRunMetadataDict;
onLaunch: (style: ReExecutionStyle) => Promise<void>;
}

export const CancelRunButton: React.FC<{run: RunFragment}> = ({run}) => {
Expand Down Expand Up @@ -102,24 +104,40 @@ export const canRunFromFailure = (run: RunFragment) =>
run.executionPlan && failedStatuses.has(run.status);

export const RunActionButtons: React.FC<RunActionButtonsProps> = (props) => {
const {metadata, graph, onLaunch, run} = props;
const artifactsPersisted = run?.executionPlan?.artifactsPersisted;
const {metadata, graph, run} = props;

const repoMatch = useRepositoryForRunWithParentSnapshot(run);
const jobError = useJobAvailabilityErrorForRun(run);

const selection = stepSelectionWithState(props.selection, metadata);
const artifactsPersisted = run?.executionPlan?.artifactsPersisted;

const selection = stepSelectionWithState(props.selection, metadata);
const currentRunSelection = stepSelectionFromRunTags(run, graph, metadata);
const currentRunIsFromFailure = run.tags?.some(
(t) => t.key === DagsterTag.IsResumeRetry && t.value === 'true',
);

const reexecute = useJobReexecution();
const reexecuteWithSelection = async (selection: StepSelection) => {
if (!run || !repoMatch || !run.pipelineSnapshotId) {
return;
}
const executionParams = getReexecutionParamsForSelection({
run,
selection,
repositoryLocationName: repoMatch.match.repositoryLocation.name,
repositoryName: repoMatch.match.repository.name,
});
await reexecute(run, executionParams);
};

const full: LaunchButtonConfiguration = {
icon: 'cached',
scope: '*',
title: 'All steps in root run',
tooltip: 'Re-execute the pipeline run from scratch',
disabled: !canRunAllSteps(run),
onClick: () => onLaunch({type: 'all'}),
onClick: () => reexecute(run, ReexecutionStrategy.ALL_STEPS),
};

const same: LaunchButtonConfiguration = {
Expand All @@ -137,7 +155,7 @@ export const RunActionButtons: React.FC<RunActionButtonsProps> = (props) => {
<StepSelectionDescription selection={currentRunSelection} />
</div>
),
onClick: () => onLaunch({type: 'selection', selection: currentRunSelection!}),
onClick: () => reexecuteWithSelection(currentRunSelection!),
};

const selected: LaunchButtonConfiguration = {
Expand All @@ -155,15 +173,15 @@ export const RunActionButtons: React.FC<RunActionButtonsProps> = (props) => {
<StepSelectionDescription selection={selection} />
</div>
),
onClick: () => onLaunch({type: 'selection', selection}),
onClick: () => reexecuteWithSelection(selection),
};

const fromSelected: LaunchButtonConfiguration = {
icon: 'arrow_forward',
title: 'From selected',
disabled: !canRunAllSteps(run) || selection.keys.length !== 1,
tooltip: 'Re-execute the pipeline downstream from the selected steps',
onClick: () => {
onClick: async () => {
if (!run.executionPlan) {
console.warn('Run execution plan must be present to launch from-selected execution');
return Promise.resolve();
Expand All @@ -173,12 +191,9 @@ export const RunActionButtons: React.FC<RunActionButtonsProps> = (props) => {
(node) => node.name,
);

return onLaunch({
type: 'selection',
selection: {
keys: selectionKeys,
query: selectionAndDownstreamQuery,
},
await reexecuteWithSelection({
keys: selectionKeys,
query: selectionAndDownstreamQuery,
});
},
};
Expand All @@ -192,7 +207,7 @@ export const RunActionButtons: React.FC<RunActionButtonsProps> = (props) => {
tooltip: !fromFailureEnabled
? 'Retry is only enabled when the pipeline has failed.'
: 'Retry the pipeline run, skipping steps that completed successfully',
onClick: () => onLaunch({type: 'from-failure'}),
onClick: () => reexecute(run, ReexecutionStrategy.FROM_FAILURE),
};

if (!artifactsPersisted) {
Expand Down
45 changes: 7 additions & 38 deletions js_modules/dagster-ui/packages/ui-core/src/runs/RunActionsMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {gql, useLazyQuery, useMutation} from '@apollo/client';
import {gql, useLazyQuery} from '@apollo/client';
import {
Button,
Icon,
Expand All @@ -17,7 +17,7 @@ import {
StyledRawCodeMirror,
} from '@dagster-io/ui-components';
import * as React from 'react';
import {Link, useHistory} from 'react-router-dom';
import {Link} from 'react-router-dom';
import styled from 'styled-components';

import {AppContext} from '../app/AppContext';
Expand All @@ -37,24 +37,16 @@ import {DeletionDialog} from './DeletionDialog';
import {ReexecutionDialog} from './ReexecutionDialog';
import {doneStatuses, failedStatuses} from './RunStatuses';
import {RunTags} from './RunTags';
import {
LAUNCH_PIPELINE_REEXECUTION_MUTATION,
RunsQueryRefetchContext,
getReexecutionVariables,
handleLaunchResult,
} from './RunUtils';
import {RunsQueryRefetchContext} from './RunUtils';
import {RunFilterToken} from './RunsFilterInput';
import {TerminationDialog} from './TerminationDialog';
import {
PipelineEnvironmentQuery,
PipelineEnvironmentQueryVariables,
} from './types/RunActionsMenu.types';
import {RunTableRunFragment} from './types/RunTable.types';
import {
LaunchPipelineReexecutionMutation,
LaunchPipelineReexecutionMutationVariables,
} from './types/RunUtils.types';
import {useJobAvailabilityErrorForRun} from './useJobAvailabilityErrorForRun';
import {useJobReexecution} from './useJobReExecution';

export const RunActionsMenu: React.FC<{
run: RunTableRunFragment;
Expand All @@ -67,16 +59,9 @@ export const RunActionsMenu: React.FC<{
>('none');

const {rootServerURI} = React.useContext(AppContext);
const history = useHistory();

const copyConfig = useCopyToClipboard();

const [reexecute] = useMutation<
LaunchPipelineReexecutionMutation,
LaunchPipelineReexecutionMutationVariables
>(LAUNCH_PIPELINE_REEXECUTION_MUTATION, {
onCompleted: refetch,
});
const reexecute = useJobReexecution({onCompleted: refetch});

const [loadEnv, {called, loading, data}] = useLazyQuery<
PipelineEnvironmentQuery,
Expand Down Expand Up @@ -130,6 +115,7 @@ export const RunActionsMenu: React.FC<{
<Menu>
<MenuItem
tagName="button"
style={{minWidth: 200}}
text={loading ? 'Loading configuration...' : 'View configuration...'}
disabled={!runConfigYaml}
icon="open_in_new"
Expand Down Expand Up @@ -205,24 +191,7 @@ export const RunActionsMenu: React.FC<{
disabled={reexecutionDisabledState.disabled}
icon="refresh"
onClick={async () => {
if (repoMatch && runConfigYaml) {
const result = await reexecute({
variables: getReexecutionVariables({
run: {...run, runConfigYaml},
style: {type: 'all'},
repositoryLocationName: repoMatch.match.repositoryLocation.name,
repositoryName: repoMatch.match.repository.name,
}),
});
handleLaunchResult(
run.pipelineName,
result.data?.launchPipelineReexecution,
history,
{
behavior: 'open',
},
);
}
await reexecute(run, ReexecutionStrategy.ALL_STEPS);
}}
/>
</Tooltip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ const Row = styled.tr<{highlighted: boolean}>`
function ActionBar({top, bottom}: {top: React.ReactNode; bottom?: React.ReactNode}) {
return (
<Box flex={{direction: 'column'}} padding={{vertical: 12}}>
<Box flex={{alignItems: 'center', gap: 12}} padding={{left: 24, right: 24}}>
<Box flex={{alignItems: 'center', gap: 12}} padding={{left: 24, right: 12}}>
{top}
</Box>
{bottom ? (
Expand Down
36 changes: 10 additions & 26 deletions js_modules/dagster-ui/packages/ui-core/src/runs/RunUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,22 +144,13 @@ function getBaseExecutionMetadata(run: RunFragment | RunTableRunFragment) {
};
}

export type ReExecutionStyle =
| {type: 'all'}
| {type: 'from-failure'}
| {type: 'selection'; selection: StepSelection};

export function getReexecutionVariables(input: {
export function getReexecutionParamsForSelection(input: {
run: (RunFragment | RunTableRunFragment) & {runConfigYaml: string};
style: ReExecutionStyle;
selection: StepSelection;
repositoryLocationName: string;
repositoryName: string;
}) {
const {run, style, repositoryLocationName, repositoryName} = input;

if (!run || !run.pipelineSnapshotId) {
return undefined;
}
const {run, selection, repositoryLocationName, repositoryName} = input;

const executionParams: ExecutionParams = {
mode: run.mode,
Expand All @@ -178,20 +169,13 @@ export function getReexecutionVariables(input: {
},
};

if (style.type === 'from-failure') {
executionParams.executionMetadata?.tags?.push({
key: DagsterTag.IsResumeRetry,
value: 'true',
});
}
if (style.type === 'selection') {
executionParams.stepKeys = style.selection.keys;
executionParams.executionMetadata?.tags?.push({
key: DagsterTag.StepSelection,
value: style.selection.query,
});
}
return {executionParams};
executionParams.stepKeys = selection.keys;
executionParams.executionMetadata?.tags?.push({
key: DagsterTag.StepSelection,
value: selection.query,
});

return executionParams;
}

export const LAUNCH_PIPELINE_EXECUTION_MUTATION = gql`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export const RunsRoot = () => {
{currentTab === 'queued' && canSeeConfig ? (
<Box
flex={{direction: 'column', gap: 8}}
padding={{horizontal: 24, vertical: 16}}
padding={{left: 24, right: 12, vertical: 16}}
border="bottom"
>
<Alert
Expand Down
Loading

1 comment on commit 47ad505

@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-192c05y68-elementl.vercel.app

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

Please sign in to comment.