From 840ee1fcc2c29b53412f0fef9b707941b7c6741a Mon Sep 17 00:00:00 2001 From: David Liu <48995019+dliu27@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:26:31 -0500 Subject: [PATCH] [ui] use jobName as fallback if only launching 1 job (#26597) ## Summary & Motivation https://dagsterlabs.slack.com/archives/C06QU0WBCQ5/p1734625511029539 ## How I Tested These Changes Will test in dogfood replicating this situation --- .../useLaunchMultipleRunsWithTelemetry.ts | 15 +- .../ui-core/src/runs/RunConfigDialog.tsx | 20 ++- .../ui-core/src/ticks/DryRunRequestTable.tsx | 1 + .../src/ticks/EvaluateScheduleDialog.tsx | 11 +- .../ui-core/src/ticks/SensorDryRunDialog.tsx | 3 +- .../EvaluateScheduleDialog.fixtures.tsx | 138 ++++++++++++++++++ .../SensorDryRunDialog.fixtures.tsx | 104 +++++++++++++ .../__tests__/EvaluateScheduleDialog.test.tsx | 47 ++++++ .../__tests__/SensorDryRunDialog.test.tsx | 48 +++++- 9 files changed, 377 insertions(+), 10 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/launchpad/useLaunchMultipleRunsWithTelemetry.ts b/js_modules/dagster-ui/packages/ui-core/src/launchpad/useLaunchMultipleRunsWithTelemetry.ts index 9249da1025e91..77b1891f512d6 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/launchpad/useLaunchMultipleRunsWithTelemetry.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/launchpad/useLaunchMultipleRunsWithTelemetry.ts @@ -24,15 +24,26 @@ export function useLaunchMultipleRunsWithTelemetry() { const history = useHistory(); return useCallback( - async (variables: LaunchMultipleRunsMutationVariables, behavior: LaunchBehavior) => { + async ( + variables: LaunchMultipleRunsMutationVariables, + behavior: LaunchBehavior, + jobName: string, + ) => { try { const executionParamsList = Array.isArray(variables.executionParamsList) ? variables.executionParamsList : [variables.executionParamsList]; - const jobNames = executionParamsList.map( + + let jobNames = executionParamsList.map( (params) => params.selector.jobName || params.selector.pipelineName, ); + // if only executing one job, and jobName isn't defined, fallback to jobName from sensor/schedule + if (executionParamsList.length === 1 && !executionParamsList[0]?.selector?.jobName) { + jobNames = [jobName]; + executionParamsList[0]!.selector.jobName = jobName; + } + if ( jobNames.length !== executionParamsList.length || jobNames.includes(undefined) || diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/RunConfigDialog.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/RunConfigDialog.tsx index 7dad84a6c971d..713a6650a95f2 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/RunConfigDialog.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/RunConfigDialog.tsx @@ -17,7 +17,7 @@ interface Props { runConfigYaml: string; mode: string | null; isJob: boolean; - + jobName?: string; // Optionally provide tags to display them as well. tags?: RunTagsFragment[]; @@ -27,8 +27,18 @@ interface Props { } export const RunConfigDialog = (props: Props) => { - const {isOpen, onClose, copyConfig, runConfigYaml, tags, mode, isJob, request, repoAddress} = - props; + const { + isOpen, + onClose, + copyConfig, + runConfigYaml, + tags, + mode, + isJob, + jobName, + request, + repoAddress, + } = props; const hasTags = !!tags && tags.length > 0; return ( @@ -76,10 +86,12 @@ export const RunConfigDialog = (props: Props) => { topBorder left={ request && - repoAddress && ( + repoAddress && + jobName && ( diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/DryRunRequestTable.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/DryRunRequestTable.tsx index b9859d06be68a..ca7e773b9c6e1 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/DryRunRequestTable.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/DryRunRequestTable.tsx @@ -71,6 +71,7 @@ export const RunRequestTable = ({runRequests, isJob, repoAddress, mode, jobName} runConfigYaml={selectedRequest.runConfigYaml} tags={selectedRequest.tags} isJob={isJob} + jobName={jobName} request={selectedRequest} repoAddress={repoAddress} /> diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/EvaluateScheduleDialog.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/EvaluateScheduleDialog.tsx index 7262979f9473a..4cb739a3d7447 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/EvaluateScheduleDialog.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/EvaluateScheduleDialog.tsx @@ -190,7 +190,7 @@ const EvaluateSchedule = ({repoAddress, name, onClose, jobName}: Props) => { try { if (executionParamsList) { - await launchMultipleRunsWithTelemetry({executionParamsList}, 'toast'); + await launchMultipleRunsWithTelemetry({executionParamsList}, 'toast', jobName); } } catch (e) { console.error(e); @@ -198,7 +198,14 @@ const EvaluateSchedule = ({repoAddress, name, onClose, jobName}: Props) => { setLaunching(false); onClose(); - }, [canLaunchAll, executionParamsList, launchMultipleRunsWithTelemetry, onClose, trackEvent]); + }, [ + canLaunchAll, + executionParamsList, + jobName, + launchMultipleRunsWithTelemetry, + onClose, + trackEvent, + ]); const content = useMemo(() => { // launching all runs state diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/SensorDryRunDialog.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/SensorDryRunDialog.tsx index 1047d13d9a391..83cad592e15a7 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/SensorDryRunDialog.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/SensorDryRunDialog.tsx @@ -196,7 +196,7 @@ const SensorDryRun = ({repoAddress, name, currentCursor, onClose, jobName}: Prop try { if (executionParamsList) { - await launchMultipleRunsWithTelemetry({executionParamsList}, 'toast'); + await launchMultipleRunsWithTelemetry({executionParamsList}, 'toast', jobName); onCommitTickResult(); // persist tick } } catch (e) { @@ -208,6 +208,7 @@ const SensorDryRun = ({repoAddress, name, currentCursor, onClose, jobName}: Prop }, [ canLaunchAll, executionParamsList, + jobName, launchMultipleRunsWithTelemetry, onClose, onCommitTickResult, diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/EvaluateScheduleDialog.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/EvaluateScheduleDialog.fixtures.tsx index 724b62da00125..da76071aa8ea9 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/EvaluateScheduleDialog.fixtures.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/EvaluateScheduleDialog.fixtures.tsx @@ -79,6 +79,44 @@ export const scheduleDryWithWithRunRequest = { }), }; +export const scheduleDryWithWithRunRequestUndefinedName = { + __typename: 'Mutation' as const, + scheduleDryRun: buildDryRunInstigationTick({ + timestamp: 1674950400, + evaluationResult: buildTickEvaluation({ + runRequests: [ + buildRunRequest({ + jobName: undefined, + runConfigYaml: + 'ops:\n configurable_op:\n config:\n scheduled_date: 2023-01-29\n', + tags: [ + buildPipelineTag({ + key: 'dagster/schedule_name', + value: 'configurable_job_schedule', + }), + buildPipelineTag({ + key: 'date', + value: '2023-01-29', + __typename: 'PipelineTag' as const, + }), + buildPipelineTag({ + key: 'github_test', + value: 'test', + }), + buildPipelineTag({ + key: 'okay_t2', + value: 'okay', + }), + ], + runKey: 'EvaluateScheduleDialog.test.tsx:1675705668.993122345', + }), + ], + skipReason: null, + error: null, + }), + }), +}; + export const ScheduleDryRunMutationRunRequests: MockedResponse = { request: { query: SCHEDULE_DRY_RUN_MUTATION, @@ -94,6 +132,22 @@ export const ScheduleDryRunMutationRunRequests: MockedResponse = + { + request: { + query: SCHEDULE_DRY_RUN_MUTATION, + variables: { + selectorData: { + scheduleName: 'test', + repositoryLocationName: 'testLocation', + repositoryName: 'testName', + }, + timestamp: 5, + }, + }, + result: {data: scheduleDryWithWithRunRequestUndefinedName}, + }; + export const ScheduleDryRunMutationError: MockedResponse = { request: { query: SCHEDULE_DRY_RUN_MUTATION, @@ -253,3 +307,87 @@ export const ScheduleLaunchAllMutation: MockedResponse = + { + request: { + query: LAUNCH_MULTIPLE_RUNS_MUTATION, + variables: { + executionParamsList: [ + { + runConfigData: + 'ops:\n configurable_op:\n config:\n scheduled_date: 2023-01-29', + selector: { + jobName: 'testJobName', // fallback + repositoryLocationName: 'testLocation', + repositoryName: 'testName', + assetSelection: [], + assetCheckSelection: [], + solidSelection: undefined, + }, + mode: 'default', + executionMetadata: { + tags: [ + { + key: 'dagster/schedule_name', + value: 'configurable_job_schedule', + }, + { + key: 'date', + value: '2023-01-29', + }, + { + key: 'github_test', + value: 'test', + }, + { + key: 'okay_t2', + value: 'okay', + }, + ], + }, + }, + ], + }, + }, + result: { + data: { + __typename: 'Mutation', + launchMultipleRuns: buildLaunchMultipleRunsResult({ + launchMultipleRunsResult: [ + buildLaunchRunSuccess({ + run: buildRun({ + id: '504b3a77-d6c4-440c-a128-7f59c9d75d59', + pipeline: buildPipelineSnapshot({ + name: 'testJobName', // fallback + }), + tags: [ + buildPipelineTag({ + key: 'dagster/schedule_name', + value: 'configurable_job_schedule', + }), + buildPipelineTag({ + key: 'date', + value: '2023-01-29', + }), + buildPipelineTag({ + key: 'github_test', + value: 'test', + }), + buildPipelineTag({ + key: 'okay_t2', + value: 'okay', + }), + ], + status: RunStatus.QUEUED, + runConfigYaml: + 'ops:\n configurable_op:\n config:\n scheduled_date: 2023-01-29', + mode: 'default', + resolvedOpSelection: null, + }), + }), + ], + }), + }, + }, + }; diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/SensorDryRunDialog.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/SensorDryRunDialog.fixtures.tsx index 98e691e5086a9..68d62e5701303 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/SensorDryRunDialog.fixtures.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/__fixtures__/SensorDryRunDialog.fixtures.tsx @@ -55,6 +55,19 @@ export const runRequests: RunRequest[] = [ }), ]; +export const runRequestWithUndefinedJobName: RunRequest[] = [ + buildRunRequest({ + jobName: undefined, // undefined jobName + runKey: 'DryRunRequestTable.test.tsx:1675705668.9931223', + runConfigYaml: + 'solids:\n read_file:\n config:\n directory: /Users/marcosalazar/code/dagster/js_modules/dagster-ui/packages/ui-core/src/ticks/tests\n filename: DryRunRequestTable.test.tsx\n', + tags: [ + buildPipelineTag({key: 'dagster2', value: 'test'}), + buildPipelineTag({key: 'marco2', value: 'salazar2'}), + ], + }), +]; + export const SensorDryRunMutationRunRequests: MockedResponse = { request: { query: EVALUATE_SENSOR_MUTATION, @@ -81,6 +94,33 @@ export const SensorDryRunMutationRunRequests: MockedResponse = + { + request: { + query: EVALUATE_SENSOR_MUTATION, + variables: { + selectorData: { + sensorName: 'test', + repositoryLocationName: 'testLocation', + repositoryName: 'testName', + }, + cursor: 'testCursortesting123', + }, + }, + result: { + data: { + __typename: 'Mutation', + sensorDryRun: buildDryRunInstigationTick({ + evaluationResult: buildTickEvaluation({ + cursor: 'a new cursor', + runRequests: runRequestWithUndefinedJobName, + error: null, + }), + }), + }, + }, + }; + export const SensorDryRunMutationError: MockedResponse = { request: { query: EVALUATE_SENSOR_MUTATION, @@ -335,3 +375,67 @@ export const SensorLaunchAllMutation: MockedResponse }, }, }; + +export const SensorLaunchAllMutation1JobWithUndefinedJobName: MockedResponse = + { + request: { + query: LAUNCH_MULTIPLE_RUNS_MUTATION, + variables: { + executionParamsList: [ + { + runConfigData: + 'solids:\n read_file:\n config:\n directory: /Users/marcosalazar/code/dagster/js_modules/dagster-ui/packages/ui-core/src/ticks/tests\n filename: DryRunRequestTable.test.tsx', + selector: { + jobName: 'testJobName', // fallback + repositoryLocationName: 'testLocation', + repositoryName: 'testName', + assetSelection: [], + assetCheckSelection: [], + solidSelection: undefined, + }, + mode: 'default', + executionMetadata: { + tags: [ + { + key: 'dagster2', + value: 'test', + }, + { + key: 'marco2', + value: 'salazar2', + }, + ], + }, + }, + ], + }, + }, + result: { + data: { + __typename: 'Mutation', + launchMultipleRuns: buildLaunchMultipleRunsResult({ + launchMultipleRunsResult: [ + buildLaunchRunSuccess({ + __typename: 'LaunchRunSuccess', + run: buildRun({ + __typename: 'Run', + id: '504b3a77-d6c4-440c-a128-7f59c9d75d59', + pipeline: buildPipelineSnapshot({ + name: 'testJobName', + }), + tags: [ + buildPipelineTag({key: 'dagster2', value: 'test'}), + buildPipelineTag({key: 'marco2', value: 'salazar2'}), + ], + status: RunStatus.QUEUED, + runConfigYaml: + 'solids:\n read_file:\n config:\n directory: /Users/marcosalazar/code/dagster/js_modules/dagster-ui/packages/ui-core/src/ticks/tests\n filename: DryRunRequestTable.test.tsx\n', + mode: 'default', + resolvedOpSelection: null, + }), + }), + ], + }), + }, + }, + }; diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/EvaluateScheduleDialog.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/EvaluateScheduleDialog.test.tsx index a8a2b55c06eb5..10eb96d2d2f3d 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/EvaluateScheduleDialog.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/EvaluateScheduleDialog.test.tsx @@ -10,8 +10,10 @@ import { GetScheduleQueryMock, ScheduleDryRunMutationError, ScheduleDryRunMutationRunRequests, + ScheduleDryRunMutationRunRequestsWithUndefinedName, ScheduleDryRunMutationSkipped, ScheduleLaunchAllMutation, + ScheduleLaunchAllMutationWithUndefinedName, } from '../__fixtures__/EvaluateScheduleDialog.fixtures'; // This component is unit tested separately so mocking it out @@ -160,4 +162,49 @@ describe('EvaluateScheduleTest', () => { expect(pushSpy).toHaveBeenCalled(); }); }); + + it('launches all runs for 1 runrequest with undefined job name in the runrequest', async () => { + const pushSpy = jest.fn(); + const createHrefSpy = jest.fn(); + + (useHistory as jest.Mock).mockReturnValue({ + push: pushSpy, + createHref: createHrefSpy, + }); + + (useTrackEvent as jest.Mock).mockReturnValue(jest.fn()); + + render( + + + , + ); + const selectButton = await screen.findByTestId('tick-selection'); + await userEvent.click(selectButton); + await waitFor(() => { + expect(screen.getByTestId('tick-5')).toBeVisible(); + }); + await userEvent.click(screen.getByTestId('tick-5')); + await userEvent.click(screen.getByTestId('continue')); + await waitFor(() => { + expect(screen.getByText(/1\s+run request/i)).toBeVisible(); + expect(screen.getByTestId('launch-all')).not.toBeDisabled(); + }); + + userEvent.click(screen.getByTestId('launch-all')); + + await waitFor(() => { + expect(screen.getByText(/Launching runs/i)).toBeVisible(); + }); + + await waitFor(() => { + expect(pushSpy).toHaveBeenCalled(); + }); + }); }); diff --git a/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/SensorDryRunDialog.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/SensorDryRunDialog.test.tsx index efb95aa30ed1e..b217a38741e75 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/SensorDryRunDialog.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/ticks/__tests__/SensorDryRunDialog.test.tsx @@ -95,7 +95,7 @@ describe('SensorDryRunTest', () => { expect(screen.getByTestId('cursor-input')).toBeVisible(); }); - it('launches all runs', async () => { + it('launches all runs with well defined job names', async () => { const pushSpy = jest.fn(); const createHrefSpy = jest.fn(); @@ -140,4 +140,50 @@ describe('SensorDryRunTest', () => { expect(pushSpy).toHaveBeenCalled(); }); }); + + it('launches all runs for 1 runrequest with undefined job name in the runrequest', async () => { + const pushSpy = jest.fn(); + const createHrefSpy = jest.fn(); + + (useHistory as jest.Mock).mockReturnValue({ + push: pushSpy, + createHref: createHrefSpy, + }); + + (useTrackEvent as jest.Mock).mockReturnValue(jest.fn()); + + render( + + + , + ); + const cursorInput = await screen.findByTestId('cursor-input'); + await userEvent.type(cursorInput, 'testing123'); + await userEvent.click(screen.getByTestId('continue')); + await waitFor(() => { + expect(screen.getByText(/1\srun requests/g)).toBeVisible(); + expect(screen.queryByText('Skipped')).toBe(null); + expect(screen.queryByText('Failed')).toBe(null); + }); + + await waitFor(() => { + expect(screen.getByTestId('launch-all')).not.toBeDisabled(); + }); + + userEvent.click(screen.getByTestId('launch-all')); + + await waitFor(() => { + expect(screen.getByText(/Launching runs/i)).toBeVisible(); + }); + + await waitFor(() => { + expect(pushSpy).toHaveBeenCalled(); + }); + }); });