Skip to content

Commit

Permalink
[ui] Remove ability to launch an asset backfill as a single run (#16737)
Browse files Browse the repository at this point in the history 
Related discussion: #16708

## Summary & Motivation

- This feature is confusing and only works if you've configured your
assets correctly, and we now have a better solution! [See discussion
above]

## How I Tested These Changes

This is a pretty clean removal and we have test coverage for this modal!
Had to remove the test covering this behavior.

---------

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
@bengotow
bengotow and bengotow authored Sep 25, 2023
1 parent 47ad505 commit 73123b5
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 104 deletions.
70 changes: 3 additions & 67 deletions js_modules/dagster-ui/packages/ui-core/src/assets/LaunchAssetChoosePartitionsDialog.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {gql, useApolloClient, useQuery} from '@apollo/client';
// eslint-disable-next-line no-restricted-imports
import {Radio} from '@blueprintjs/core';
import {
Box,
Button,
Expand All @@ -14,7 +13,6 @@ import {
Checkbox,
Icon,
Subheading,
RadioContainer,
} from '@dagster-io/ui-components';
import reject from 'lodash/reject';
import React from 'react';
Expand Down Expand Up @@ -54,7 +52,6 @@ import {
} from '../partitions/BackfillMessaging';
import {DimensionRangeWizard} from '../partitions/DimensionRangeWizard';
import {assembleIntoSpans, stringForSpan} from '../partitions/SpanRepresentation';
import {DagsterTag} from '../runs/RunTag';
import {testId} from '../testing/testId';
import {RepoAddress} from '../workspace/types';

Expand Down Expand Up @@ -195,11 +192,6 @@ const LaunchAssetChoosePartitionsDialogBody: React.FC<Props> = ({
shouldReadPartitionQueryStringParam: true,
});

const [launchWithRangesAsTags, setLaunchWithRangesAsTags] = React.useState(false);
const canLaunchWithRangesAsTags =
selections.every((s) => s.selectedRanges.length === 1) &&
selections.some((s) => s.selectedKeys.length > 1);

const keysFiltered = React.useMemo(() => {
return explodePartitionKeysInSelectionMatching(selections, (dIdxs) => {
if (missingFailedOnly) {
Expand All @@ -218,16 +210,7 @@ const LaunchAssetChoosePartitionsDialogBody: React.FC<Props> = ({
const {useLaunchWithTelemetry} = useLaunchPadHooks();
const launchWithTelemetry = useLaunchWithTelemetry();
const launchAsBackfill =
['pureWithAnchorAsset', 'pureAll'].includes(target.type) ||
(!launchWithRangesAsTags && keysFiltered.length !== 1);

React.useEffect(() => {
!canLaunchWithRangesAsTags && setLaunchWithRangesAsTags(false);
}, [canLaunchWithRangesAsTags]);

React.useEffect(() => {
launchWithRangesAsTags && setMissingFailedOnly(false);
}, [launchWithRangesAsTags]);
['pureWithAnchorAsset', 'pureAll'].includes(target.type) || keysFiltered.length !== 1;

React.useEffect(() => {
['pureWithAnchorAsset', 'pureAll'].includes(target.type) && setMissingFailedOnly(false);
Expand Down Expand Up @@ -307,19 +290,7 @@ const LaunchAssetChoosePartitionsDialogBody: React.FC<Props> = ({
}

const runConfigData = partition.runConfigOrError.yaml || '';
let allTags = [...partition.tagsOrError.results, ...tags];

if (launchWithRangesAsTags) {
allTags = allTags.filter((t) => !t.key.startsWith(DagsterTag.Partition));
allTags.push({
key: DagsterTag.AssetPartitionRangeStart,
value: keysFiltered[0]!,
});
allTags.push({
key: DagsterTag.AssetPartitionRangeEnd,
value: keysFiltered[keysFiltered.length - 1]!,
});
}
const allTags = [...partition.tagsOrError.results, ...tags];

const result = await launchWithTelemetry(
{
Expand Down Expand Up @@ -578,48 +549,13 @@ const LaunchAssetChoosePartitionsDialogBody: React.FC<Props> = ({
isInitiallyOpen={true}
>
{target.type === 'job' && (
<Box padding={{vertical: 16, horizontal: 24}} flex={{direction: 'column', gap: 12}}>
<Box padding={{vertical: 16, horizontal: 24}}>
<Checkbox
data-testid={testId('missing-only-checkbox')}
label="Backfill only failed and missing partitions within selection"
checked={missingFailedOnly}
disabled={launchWithRangesAsTags}
onChange={() => setMissingFailedOnly(!missingFailedOnly)}
/>
<RadioContainer>
<Subheading>Launch as...</Subheading>
<Radio
data-testid={testId('ranges-as-tags-true-radio')}
checked={canLaunchWithRangesAsTags && launchWithRangesAsTags}
disabled={!canLaunchWithRangesAsTags}
onChange={() => setLaunchWithRangesAsTags(!launchWithRangesAsTags)}
>
<Box flex={{direction: 'row', alignItems: 'center', gap: 8}}>
<span>Single run</span>
<Tooltip
targetTagName="div"
position="top-left"
content={
<div style={{maxWidth: 300}}>
This option requires that your assets are written to operate on a
partition key range via context.asset_partition_key_range_for_output or
context.asset_partitions_time_window_for_output.
</div>
}
>
<Icon name="info" color={Colors.Gray500} />
</Tooltip>
</Box>
</Radio>
<Radio
data-testid={testId('ranges-as-tags-false-radio')}
checked={!canLaunchWithRangesAsTags || !launchWithRangesAsTags}
disabled={!canLaunchWithRangesAsTags}
onChange={() => setLaunchWithRangesAsTags(!launchWithRangesAsTags)}
>
Multiple runs (One per selected partition)
</Radio>
</RadioContainer>
</Box>
)}
</ToggleableSection>
Expand Down
37 changes: 0 additions & 37 deletions ...ules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,43 +325,8 @@ describe('LaunchAssetExecutionButton', () => {
// missing-and-failed only option is available
expect(screen.getByTestId('missing-only-checkbox')).toBeEnabled();

// ranges-as-tags option is available
const rangesAsTags = screen.getByTestId('ranges-as-tags-true-radio');
await waitFor(async () => expect(rangesAsTags).toBeEnabled());

await expectLaunchExecutesMutationAndCloses('Launch 1148-run backfill', launchMock);
});

it('should launch a single run if you choose to pass the partition range using tags', async () => {
const launchMock = buildExpectedLaunchSingleRunMutation({
mode: 'default',
executionMetadata: {
tags: [
{key: 'dagster/asset_partition_range_start', value: '2020-01-02'},
{key: 'dagster/asset_partition_range_end', value: '2023-02-22'},
],
},
runConfigData: '{}\n',
selector: {
repositoryLocationName: 'test.py',
repositoryName: 'repo',
pipelineName: 'my_asset_job',
assetSelection: [{path: ['asset_daily']}],
},
});
renderButton({
scope: {all: [ASSET_DAILY]},
preferredJobName: 'my_asset_job',
launchMock,
});
await clickMaterializeButton();
await screen.findByTestId('choose-partitions-dialog');

const rangesAsTags = screen.getByTestId('ranges-as-tags-true-radio');
await waitFor(async () => expect(rangesAsTags).toBeEnabled());
await userEvent.click(rangesAsTags);
await expectLaunchExecutesMutationAndCloses('Launch 1 run', launchMock);
});
});

describe('partition mapped assets', () => {
Expand Down Expand Up @@ -389,7 +354,6 @@ describe('LaunchAssetExecutionButton', () => {

// backfill options for run as tags, missing only are not available
expect(screen.queryByTestId('missing-only-checkbox')).toBeNull();
expect(screen.queryByTestId('ranges-as-tags-true-radio')).toBeNull();

await expectLaunchExecutesMutationAndCloses('Launch backfill', LaunchMutationMock);
});
Expand Down Expand Up @@ -420,7 +384,6 @@ describe('LaunchAssetExecutionButton', () => {

// backfill options for run as tags, missing only are not available
expect(await screen.queryByTestId('missing-only-checkbox')).toBeNull();
expect(await screen.queryByTestId('ranges-as-tags-true-radio')).toBeNull();

await expectLaunchExecutesMutationAndCloses('Launch backfill', LaunchPureAllMutationMock);
});
Expand Down

1 comment on commit 73123b5

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

Built with commit 73123b5.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.