Skip to content

Commit

Permalink
[ui] Include assets with failed/missing partitions in “Materialize un…
Browse files Browse the repository at this point in the history 
…synced" (#20985)

Fixes #20463

## Summary & Motivation

This PR updates the behavior of "Materialize unsynced". In addition to
assets with the staleStatus = MISSING, we now include assets with
missing or failed partitions.

I also adjusted the types a bit in these code paths -- because
`staleCauses` can be undefined in the types passed to `isAssetStale`,
you could technically call it with "include=self" and never get anything
but false, which felt a little scary. I broke the function apart into
two with stricter types.

## How I Tested These Changes

Tested with a graph that includes some partially materialized
partitioned assets!

---------

Co-authored-by: bengotow <[email protected]>
  • Loading branch information
@bengotow
bengotow and bengotow authored Apr 3, 2024
1 parent 9ad6b22 commit 3d206d5
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 46 deletions.
4 changes: 2 additions & 2 deletions js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNode.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {withMiddleTruncation} from '../app/Util';
import {useAssetLiveData} from '../asset-data/AssetLiveDataProvider';
import {PartitionCountTags} from '../assets/AssetNodePartitionCounts';
import {ChangedReasonsTag, MinimalNodeChangedDot} from '../assets/ChangedReasons';
import {MinimalNodeStaleDot, StaleReasonsTag, isAssetStale} from '../assets/Stale';
import {MinimalNodeStaleDot, StaleReasonsTag, isAssetStaleFiltered} from '../assets/Stale';
import {AssetChecksStatusSummary} from '../assets/asset-checks/AssetChecksStatusSummary';
import {assetDetailsPathForKey} from '../assets/assetDetailsPathForKey';
import {AssetComputeKindTag} from '../graph/OpTags';
Expand Down Expand Up @@ -185,7 +185,7 @@ export const AssetNodeMinimal = ({
const displayName = assetKey.path[assetKey.path.length - 1]!;

const isChanged = definition.changedReasons.length;
const isStale = isAssetStale(assetKey, liveData, 'upstream');
const isStale = isAssetStaleFiltered(assetKey, liveData, 'upstream');

const queuedRuns = liveData?.unstartedRunIds.length;
const inProgressRuns = liveData?.inProgressRunIds.length;
Expand Down
12 changes: 6 additions & 6 deletions js_modules/dagster-ui/packages/ui-core/src/asset-graph/CollapsedGroupNode.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {groupAssetsByStatus} from './util';
import {CloudOSSContext} from '../app/CloudOSSContext';
import {withMiddleTruncation} from '../app/Util';
import {useAssetsLiveData} from '../asset-data/AssetLiveDataProvider';
import {CalculateChangedAndMissingDialog} from '../assets/CalculateChangedAndMissingDialog';
import {CalculateUnsyncedDialog} from '../assets/CalculateUnsyncedDialog';
import {useMaterializationAction} from '../assets/LaunchAssetExecutionButton';
import {AssetKey} from '../assets/types';
import {numberFormatter} from '../ui/formatters';
Expand Down Expand Up @@ -205,7 +205,7 @@ export const useGroupNodeContextMenu = ({
preferredJobName?: string;
}) => {
const {onClick, launchpadElement} = useMaterializationAction(preferredJobName);
const [showCalculatingChangedAndMissingDialog, setShowCalculatingChangedAndMissingDialog] =
const [showCalculatingUnsyncedDialog, setShowCalculatingUnsyncedDialog] =
React.useState<boolean>(false);

const {
Expand All @@ -229,7 +229,7 @@ export const useGroupNodeContextMenu = ({
<MenuItem
icon="changes_present"
text="Materialize unsynced"
onClick={() => setShowCalculatingChangedAndMissingDialog(true)}
onClick={() => setShowCalculatingUnsyncedDialog(true)}
/>
</>
) : null}
Expand All @@ -240,10 +240,10 @@ export const useGroupNodeContextMenu = ({
);
const dialog = (
<div>
<CalculateChangedAndMissingDialog
isOpen={!!showCalculatingChangedAndMissingDialog}
<CalculateUnsyncedDialog
isOpen={showCalculatingUnsyncedDialog}
onClose={() => {
setShowCalculatingChangedAndMissingDialog(false);
setShowCalculatingUnsyncedDialog(false);
}}
assets={assets}
onMaterializeAssets={(assets: AssetKey[], e: React.MouseEvent<any>) => {
Expand Down
5 changes: 4 additions & 1 deletion js_modules/dagster-ui/packages/ui-core/src/assets/AssetPartitionDetail.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,10 @@ export const AssetPartitionDetail = ({
<Spinner purpose="body-text" />
) : (
<Box flex={{direction: 'row', gap: 8, alignItems: 'center'}}>
<StaleReasonsTag liveData={{staleCauses, staleStatus}} assetKey={assetKey} />
<StaleReasonsTag
liveData={staleCauses && staleStatus ? {staleCauses, staleStatus} : undefined}
assetKey={assetKey}
/>
<ChangedReasonsTag changedReasons={changedReasons} assetKey={assetKey} />
</Box>
)}
Expand Down
36 changes: 21 additions & 15 deletions ...sets/CalculateChangedAndMissingDialog.tsx → ...re/src/assets/CalculateUnsyncedDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ import * as React from 'react';
import {Link} from 'react-router-dom';
import styled from 'styled-components';

import {isAssetMissing, isAssetStale} from './Stale';
import {isAssetMissing, isAssetStaleAll} from './Stale';
import {asAssetKeyInput} from './asInput';
import {assetDetailsPathForKey} from './assetDetailsPathForKey';
import {AssetKey} from './types';
import {
AssetStaleStatusQuery,
AssetStaleStatusQueryVariables,
} from './types/CalculateChangedAndMissingDialog.types';
} from './types/CalculateUnsyncedDialog.types';
import {showCustomAlert} from '../app/CustomAlertProvider';
import {displayNameForAssetKey} from '../asset-graph/Utils';
import {Container, Inner, Row} from '../ui/VirtualizedTable';

export const CalculateChangedAndMissingDialog = React.memo(
export const CalculateUnsyncedDialog = React.memo(
({
isOpen,
onClose,
Expand All @@ -52,10 +52,10 @@ export const CalculateChangedAndMissingDialog = React.memo(
},
);

const staleOrMissing = React.useMemo(
const unsynced = React.useMemo(
() =>
data?.assetNodes
.filter((node) => isAssetStale(node.assetKey, node, 'all') || isAssetMissing(node))
(data?.assetNodes || [])
.filter((node) => isAssetStaleAll(node) || isAssetMissing(node))
.map(asAssetKeyInput),
[data],
);
Expand All @@ -72,7 +72,7 @@ export const CalculateChangedAndMissingDialog = React.memo(

const containerRef = React.useRef<HTMLDivElement | null>(null);
const virtualizer = useVirtualizer({
count: staleOrMissing?.length ?? 0,
count: unsynced.length,
getScrollElement: () => containerRef.current,
estimateSize: () => 28,
});
Expand All @@ -81,8 +81,8 @@ export const CalculateChangedAndMissingDialog = React.memo(

const [checked, setChecked] = React.useState<Set<AssetKey>>(new Set());
React.useLayoutEffect(() => {
setChecked(new Set(staleOrMissing));
}, [staleOrMissing]);
setChecked(new Set(unsynced));
}, [unsynced]);

const content = () => {
if (!isOpen) {
Expand All @@ -95,19 +95,19 @@ export const CalculateChangedAndMissingDialog = React.memo(
</Box>
);
}
if (staleOrMissing?.length) {
if (unsynced.length) {
return (
<>
<RowGrid border="bottom" padding={{bottom: 8}}>
<Checkbox
id="check-all"
checked={checked.size === staleOrMissing.length}
checked={checked.size === unsynced.length}
onChange={() => {
setChecked((checked) => {
if (checked.size === staleOrMissing.length) {
if (checked.size === unsynced.length) {
return new Set();
} else {
return new Set(staleOrMissing);
return new Set(unsynced);
}
});
}}
Expand All @@ -119,7 +119,7 @@ export const CalculateChangedAndMissingDialog = React.memo(
<Container ref={containerRef} style={{maxHeight: '400px'}}>
<Inner $totalHeight={totalHeight}>
{items.map(({index, key, size, start}) => {
const item = staleOrMissing[index]!;
const item = unsynced[index]!;
return (
<Row
$height={size}
Expand Down Expand Up @@ -180,7 +180,7 @@ export const CalculateChangedAndMissingDialog = React.memo(
<DialogFooter topBorder>
{loading ? (
<Button onClick={onClose}>Cancel</Button>
) : staleOrMissing?.length ? (
) : unsynced.length ? (
<Button
intent="primary"
onClick={(e) => {
Expand Down Expand Up @@ -209,6 +209,12 @@ const ASSET_STALE_STATUS_QUERY = gql`
path
}
staleStatus
partitionStats {
numMaterialized
numMaterializing
numPartitions
numFailed
}
}
}
`;
Expand Down
12 changes: 6 additions & 6 deletions js_modules/dagster-ui/packages/ui-core/src/assets/LaunchAssetExecutionButton.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
ADDITIONAL_REQUIRED_KEYS_WARNING,
MULTIPLE_DEFINITIONS_WARNING,
} from './AssetDefinedInMultipleReposNotice';
import {CalculateChangedAndMissingDialog} from './CalculateChangedAndMissingDialog';
import {CalculateUnsyncedDialog} from './CalculateUnsyncedDialog';
import {LaunchAssetChoosePartitionsDialog} from './LaunchAssetChoosePartitionsDialog';
import {partitionDefinitionsEqual} from './MultipartitioningSupport';
import {asAssetKeyInput, getAssetCheckHandleInputs} from './asInput';
Expand Down Expand Up @@ -188,7 +188,7 @@ export const LaunchAssetExecutionButton = ({

const {MaterializeButton} = useLaunchPadHooks();

const [showCalculatingChangedAndMissingDialog, setShowCalculatingChangedAndMissingDialog] =
const [showCalculatingUnsyncedDialog, setShowCalculatingUnsyncedDialog] =
React.useState<boolean>(false);

const {
Expand Down Expand Up @@ -225,10 +225,10 @@ export const LaunchAssetExecutionButton = ({

return (
<>
<CalculateChangedAndMissingDialog
isOpen={!!showCalculatingChangedAndMissingDialog}
<CalculateUnsyncedDialog
isOpen={showCalculatingUnsyncedDialog}
onClose={() => {
setShowCalculatingChangedAndMissingDialog(false);
setShowCalculatingUnsyncedDialog(false);
}}
assets={inScope}
onMaterializeAssets={(assets: AssetKey[], e: React.MouseEvent<any>) => {
Expand Down Expand Up @@ -276,7 +276,7 @@ export const LaunchAssetExecutionButton = ({
<MenuItem
text="Materialize unsynced"
icon="changes_present"
onClick={() => setShowCalculatingChangedAndMissingDialog(true)}
onClick={() => setShowCalculatingUnsyncedDialog(true)}
/>
) : null}
<MenuItem
Expand Down
54 changes: 38 additions & 16 deletions js_modules/dagster-ui/packages/ui-core/src/assets/Stale.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,42 @@ import {AssetKeyInput, StaleCauseCategory, StaleStatus} from '../graphql/types';
import {numberFormatter} from '../ui/formatters';

type StaleDataForNode = {
staleCauses?: LiveDataForNode['staleCauses'];
staleStatus?: LiveDataForNode['staleStatus'];
staleCauses: LiveDataForNode['staleCauses'];
staleStatus: LiveDataForNode['staleStatus'];

// May be omitted when showing staleness for a single partition
partitionStats?: LiveDataForNode['partitionStats'];
};
export const isAssetMissing = (liveData?: Pick<StaleDataForNode, 'staleStatus'>) =>
liveData && liveData.staleStatus === StaleStatus.MISSING;

export const isAssetStale = (
export const isAssetMissing = (
liveData?: Pick<StaleDataForNode, 'staleStatus' | 'partitionStats'>,
) => {
if (!liveData) {
return false;
}
const {partitionStats, staleStatus} = liveData;

return partitionStats
? partitionStats.numPartitions -
partitionStats.numMaterializing -
partitionStats.numMaterialized >
0
: staleStatus === StaleStatus.MISSING;
};

export const isAssetStaleAll = (liveData?: Pick<StaleDataForNode, 'staleStatus'>) => {
return liveData && liveData.staleStatus === StaleStatus.STALE;
};

export const isAssetStaleFiltered = (
assetKey: AssetKeyInput,
liveData?: Pick<StaleDataForNode, 'staleStatus'>,
include: 'all' | 'upstream' | 'self' = 'all',
liveData: Pick<StaleDataForNode, 'staleStatus' | 'staleCauses'> | undefined,
include: 'upstream' | 'self',
) => {
if (liveData && liveData.staleStatus === StaleStatus.STALE) {
if (include === 'all') {
return true;
} else {
const grouped = groupedCauses(assetKey, include, liveData);
const totalCauses = Object.values(grouped).reduce((s, g) => s + g.length, 0);
return totalCauses > 0;
}
const grouped = groupedCauses(assetKey, include, liveData);
const totalCauses = Object.values(grouped).reduce((s, g) => s + g.length, 0);
return totalCauses > 0;
}
return false;
};
Expand Down Expand Up @@ -93,7 +110,12 @@ export const StaleReasonsLabel = ({
include: 'all' | 'upstream' | 'self';
liveData?: StaleDataForNode;
}) => {
if (!isAssetStale(assetKey, liveData, include)) {
const isStale =
include === 'all'
? isAssetStaleAll(liveData)
: isAssetStaleFiltered(assetKey, liveData, include);

if (!isStale) {
return null;
}

Expand Down Expand Up @@ -185,7 +207,7 @@ export const StaleCausesPopover = ({
function groupedCauses(
assetKey: AssetKeyInput,
include: 'all' | 'upstream' | 'self',
liveData?: StaleDataForNode,
liveData?: Pick<StaleDataForNode, 'staleStatus' | 'staleCauses'>,
) {
const all = (liveData?.staleCauses || [])
.map((cause) => {
Expand Down
7 changes: 7 additions & 0 deletions ...CalculateChangedAndMissingDialog.types.ts → ...ts/types/CalculateUnsyncedDialog.types.ts
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 comment on commit 3d206d5

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

Built with commit 3d206d5.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.