Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ui] Implement new asset Automation page #26765

Merged
merged 1 commit into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {Button} from './Button';
import {Icon} from './Icon';

export interface CursorPaginationProps {
cursor?: string;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this so that the parent component can determine when the cursor has changed, and reset scroll position accordingly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I think I did this on the new RunsFeed page as well, but a bit differently. That version watches for the state to transition to loading and then back to ready (RunsFeedTable.tsx::84), but maybe that should be switched to this more direct approach too..

I think one issue I was running in to was that it looks a bit janky to scroll to the top before the data has loaded -- watching the state transition was the best way I could find to scroll only once the new data was available.

I wonder if we could have useCursorPaginatedQuery return the cursor that matches the data it loaded, and not the cursor that is is is loading, so that it behaves that way?

hasPrevCursor: boolean;
hasNextCursor: boolean;
popCursor: () => void;
Expand Down
3 changes: 2 additions & 1 deletion js_modules/dagster-ui/packages/ui-core/client.json

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

15 changes: 8 additions & 7 deletions js_modules/dagster-ui/packages/ui-core/src/assets/AssetView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@ import {AssetPartitions} from './AssetPartitions';
import {AssetPlotsPage} from './AssetPlotsPage';
import {AssetTabs} from './AssetTabs';
import {useAllAssets} from './AssetsCatalogTable';
import {AssetAutomaterializePolicyPage} from './AutoMaterializePolicyPage/AssetAutomaterializePolicyPage';
import {AssetAutomationRoot} from './AutoMaterializePolicyPage/AssetAutomationRoot';
import {ChangedReasonsTag} from './ChangedReasons';
import {LaunchAssetExecutionButton} from './LaunchAssetExecutionButton';
import {UNDERLYING_OPS_ASSET_NODE_FRAGMENT} from './UnderlyingOpsOrGraph';
import {AssetChecks} from './asset-checks/AssetChecks';
import {assetDetailsPathForKey} from './assetDetailsPathForKey';
import {AssetNodeOverview, AssetNodeOverviewNonSDA} from './overview/AssetNodeOverview';
import {AssetKey, AssetViewParams} from './types';
import {healthRefreshHintFromLiveData} from './usePartitionHealthData';
import {useReportEventsModal} from './useReportEventsModal';
import {useWipeModal} from './useWipeModal';
import {gql, useQuery} from '../apollo-client';
import {AssetTableDefinitionFragment} from './types/AssetTableFragment.types';
import {
AssetViewDefinitionNodeFragment,
AssetViewDefinitionQuery,
AssetViewDefinitionQueryVariables,
} from './types/AssetView.types';
import {useDeleteDynamicPartitionsDialog} from './useDeleteDynamicPartitionsDialog';
import {healthRefreshHintFromLiveData} from './usePartitionHealthData';
import {useReportEventsModal} from './useReportEventsModal';
import {useWipeModal} from './useWipeModal';
import {gql, useQuery} from '../apollo-client';
import {AssetTableDefinitionFragment} from './types/AssetTableFragment.types';
import {currentPageAtom} from '../app/analytics';
import {Timestamp} from '../app/time/Timestamp';
import {AssetLiveDataRefreshButton, useAssetLiveData} from '../asset-data/AssetLiveDataProvider';
Expand Down Expand Up @@ -221,7 +221,8 @@ export const AssetView = ({assetKey, headerBreadcrumbs, writeAssetVisit, current
if (isLoading) {
return <AssetLoadingDefinitionState />;
}
return <AssetAutomaterializePolicyPage assetKey={assetKey} definition={definition} />;

return <AssetAutomationRoot assetKey={assetKey} definition={definition} />;
};

const renderChecksTab = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,10 @@ export const AssetAutomaterializePolicyPage = ({
assetKey: AssetKey;
definition?: AssetViewDefinitionNodeFragment | null;
}) => {
const {queryResult, paginationProps} = useEvaluationsQueryResult({assetKey});
const {queryResult, evaluations, paginationProps} = useEvaluationsQueryResult({assetKey});

useQueryRefreshAtInterval(queryResult, FIFTEEN_SECONDS);

const evaluations = useMemo(() => {
if (
queryResult.data?.assetConditionEvaluationRecordsOrError?.__typename ===
'AssetConditionEvaluationRecords' &&
queryResult.data?.assetNodeOrError?.__typename === 'AssetNode'
) {
return queryResult.data?.assetConditionEvaluationRecordsOrError.records;
}
return [];
}, [
queryResult.data?.assetConditionEvaluationRecordsOrError,
queryResult.data?.assetNodeOrError,
]);

const isFirstPage = !paginationProps.hasPrevCursor;

const [selectedEvaluationId, setSelectedEvaluationId] = useQueryPersistedState<
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import {
Box,
CursorHistoryControls,
NonIdealState,
SpinnerWithText,
} from '@dagster-io/ui-components';
import {useEffect, useRef} from 'react';

import {useEvaluationsQueryResult} from './useEvaluationsQueryResult';
import {FIFTEEN_SECONDS, useQueryRefreshAtInterval} from '../../app/QueryRefresh';
import {AssetKey} from '../types';
import {EvaluationList} from './EvaluationList';
import {AssetViewDefinitionNodeFragment} from '../types/AssetView.types';

interface Props {
assetKey: AssetKey;
definition: AssetViewDefinitionNodeFragment | null;
}

export const AssetAutomationRoot = ({assetKey, definition}: Props) => {
const {queryResult, evaluations, paginationProps} = useEvaluationsQueryResult({assetKey});
const {data, loading} = queryResult;

const scrollRef = useRef<HTMLDivElement>(null);

// When paginating, reset scroll to top.
useEffect(() => {
if (scrollRef.current) {
scrollRef.current.scrollTo({top: 0, behavior: 'instant'});
}
}, [paginationProps.cursor]);

useQueryRefreshAtInterval(queryResult, FIFTEEN_SECONDS);

if (loading && !data) {
return (
<Box padding={64} flex={{direction: 'column', alignItems: 'center'}}>
<SpinnerWithText label="Loading evaluations…" />
</Box>
);
}

if (!definition) {
return (
<Box padding={64}>
<NonIdealState
icon="asset"
title="Asset evaluations not found"
description="This asset does not have any automation evaluations."
/>
</Box>
);
}

return (
<>
<Box
padding={{vertical: 12, horizontal: 20}}
flex={{direction: 'row', justifyContent: 'flex-end'}}
>
<CursorHistoryControls {...paginationProps} style={{marginTop: 0}} />
</Box>
<div style={{overflowY: 'auto'}} ref={scrollRef}>
<EvaluationList
evaluations={evaluations}
assetKey={assetKey}
isPartitioned={definition.partitionDefinition !== null}
/>
</div>
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,40 @@ import {
} from '@dagster-io/ui-components';
import {ReactNode, useState} from 'react';

import {GET_EVALUATIONS_QUERY} from './GetEvaluationsQuery';
import {GET_SLIM_EVALUATIONS_QUERY} from './GetEvaluationsQuery';
import {PartitionTagSelector} from './PartitionTagSelector';
import {QueryfulEvaluationDetailTable} from './QueryfulEvaluationDetailTable';
import {GetEvaluationsQuery, GetEvaluationsQueryVariables} from './types/GetEvaluationsQuery.types';
import {
GetSlimEvaluationsQuery,
GetSlimEvaluationsQueryVariables,
} from './types/GetEvaluationsQuery.types';
import {usePartitionsForAssetKey} from './usePartitionsForAssetKey';
import {useQuery} from '../../apollo-client';
import {DEFAULT_TIME_FORMAT} from '../../app/time/TimestampFormat';
import {TimestampDisplay} from '../../schedules/TimestampDisplay';

interface Props {
isOpen: boolean;
setIsOpen: (isOpen: boolean) => void;
onClose: () => void;
assetKeyPath: string[];
assetCheckName?: string;
evaluationID: string;
}

export const EvaluationDetailDialog = ({isOpen, setIsOpen, evaluationID, assetKeyPath}: Props) => {
export const EvaluationDetailDialog = ({
isOpen,
onClose,
evaluationID,
assetKeyPath,
assetCheckName,
}: Props) => {
return (
<Dialog isOpen={isOpen} onClose={() => setIsOpen(false)} style={EvaluationDetailDialogStyle}>
<Dialog isOpen={isOpen} onClose={onClose} style={EvaluationDetailDialogStyle}>
<EvaluationDetailDialogContents
evaluationID={evaluationID}
assetKeyPath={assetKeyPath}
setIsOpen={setIsOpen}
assetCheckName={assetCheckName}
onClose={onClose}
/>
</Dialog>
);
Expand All @@ -41,17 +52,26 @@ export const EvaluationDetailDialog = ({isOpen, setIsOpen, evaluationID, assetKe
interface ContentProps {
evaluationID: string;
assetKeyPath: string[];
setIsOpen: (isOpen: boolean) => void;
assetCheckName?: string;
onClose: () => void;
}

const EvaluationDetailDialogContents = ({evaluationID, assetKeyPath, setIsOpen}: ContentProps) => {
const EvaluationDetailDialogContents = ({
evaluationID,
assetKeyPath,
assetCheckName,
onClose,
}: ContentProps) => {
const [selectedPartition, setSelectedPartition] = useState<string | null>(null);

const {data, loading} = useQuery<GetEvaluationsQuery, GetEvaluationsQueryVariables>(
GET_EVALUATIONS_QUERY,
const {data, loading} = useQuery<GetSlimEvaluationsQuery, GetSlimEvaluationsQueryVariables>(
GET_SLIM_EVALUATIONS_QUERY,
{
variables: {
assetKey: {path: assetKeyPath},
assetKey: assetCheckName ? null : {path: assetKeyPath},
assetCheckKey: assetCheckName
? {assetKey: {path: assetKeyPath}, name: assetCheckName}
: null,
cursor: `${BigInt(evaluationID) + 1n}`,
limit: 2,
},
Expand All @@ -70,7 +90,7 @@ const EvaluationDetailDialogContents = ({evaluationID, assetKeyPath, setIsOpen}:
<SpinnerWithText label="Loading evaluation details..." />
</Box>
}
onDone={() => setIsOpen(false)}
onDone={onClose}
/>
);
}
Expand All @@ -90,12 +110,12 @@ const EvaluationDetailDialogContents = ({evaluationID, assetKeyPath, setIsOpen}:
/>
</Box>
}
onDone={() => setIsOpen(false)}
onDone={onClose}
/>
);
}

const evaluation = record?.records[0];
const evaluation = record?.records.find((r) => r.evaluationId === evaluationID);

if (!evaluation) {
return (
Expand All @@ -114,7 +134,7 @@ const EvaluationDetailDialogContents = ({evaluationID, assetKeyPath, setIsOpen}:
/>
</Box>
}
onDone={() => setIsOpen(false)}
onDone={onClose}
/>
);
}
Expand All @@ -135,7 +155,7 @@ const EvaluationDetailDialogContents = ({evaluationID, assetKeyPath, setIsOpen}:
</div>
}
/>
{allPartitions.length > 0 ? (
{allPartitions.length > 0 && evaluation.isLegacy ? (
<Box padding={{vertical: 12, right: 20}} flex={{justifyContent: 'flex-end'}}>
<PartitionTagSelector
allPartitions={allPartitions}
Expand All @@ -154,7 +174,7 @@ const EvaluationDetailDialogContents = ({evaluationID, assetKeyPath, setIsOpen}:
setSelectedPartition={setSelectedPartition}
/>
}
onDone={() => setIsOpen(false)}
onDone={onClose}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Table} from '@dagster-io/ui-components';
import {Colors, Table} from '@dagster-io/ui-components';

import {AssetKey} from '../types';
import {EvaluationListRow} from './EvaluationListRow';
Expand All @@ -7,13 +7,22 @@ import {AssetConditionEvaluationRecordFragment} from './types/GetEvaluationsQuer
interface Props {
assetKey: AssetKey;
isPartitioned: boolean;
assetCheckName?: string;
evaluations: AssetConditionEvaluationRecordFragment[];
}

export const EvaluationList = ({assetKey, isPartitioned, evaluations}: Props) => {
export const EvaluationList = ({assetKey, isPartitioned, assetCheckName, evaluations}: Props) => {
return (
<Table>
<thead>
<thead
style={{
position: 'sticky',
top: 0,
backgroundColor: Colors.backgroundDefault(),
zIndex: 1,
boxShadow: `inset 0 -1px 0 ${Colors.keylineDefault()}`,
}}
>
<tr>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sticky header while scrolling.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should go ahead and switch this out for a virtualized list? We have the HeaderRow for that one... With the row component this table already feels pretty close to our standard virtualized one.

    <HeaderRow templateColumns={TEMPLATE_COLUMNS} sticky>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably could be virtualized, it just seemed a bit overkill for a table with a maximum of 25-30 items.

<th>Timestamp</th>
<th style={{width: '240px'}}>Evaluation result</th>
Expand All @@ -27,6 +36,7 @@ export const EvaluationList = ({assetKey, isPartitioned, evaluations}: Props) =>
key={evaluation.id}
evaluation={evaluation}
assetKey={assetKey}
assetCheckName={assetCheckName}
isPartitioned={isPartitioned}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
Mono,
} from '@dagster-io/ui-components';
import {useState} from 'react';
import {Link} from 'react-router-dom';

import {AssetKey} from '../types';
import {EvaluationDetailDialog} from './EvaluationDetailDialog';
Expand All @@ -20,11 +21,12 @@ import {TimestampDisplay} from '../../schedules/TimestampDisplay';

interface Props {
assetKey: AssetKey;
assetCheckName?: string;
isPartitioned: boolean;
evaluation: AssetConditionEvaluationRecordFragment;
}

export const EvaluationListRow = ({evaluation, assetKey, isPartitioned}: Props) => {
export const EvaluationListRow = ({evaluation, assetKey, assetCheckName, isPartitioned}: Props) => {
const [isOpen, setIsOpen] = useState(false);

return (
Expand Down Expand Up @@ -52,9 +54,10 @@ export const EvaluationListRow = ({evaluation, assetKey, isPartitioned}: Props)
</tr>
<EvaluationDetailDialog
isOpen={isOpen}
setIsOpen={setIsOpen}
evaluationID={evaluation.id}
onClose={() => setIsOpen(false)}
evaluationID={evaluation.evaluationId}
assetKeyPath={assetKey.path}
assetCheckName={assetCheckName}
/>
</>
);
Expand All @@ -67,16 +70,28 @@ interface EvaluationRunInfoProps {

const EvaluationRunInfo = ({runIds, timestamp}: EvaluationRunInfoProps) => {
const [isOpen, setIsOpen] = useState(false);
const firstRun = runIds[0];

if (runIds.length === 0) {
if (!firstRun) {
return <span style={{color: Colors.textDisabled()}}>None</span>;
}

if (runIds.length === 1) {
const truncated = firstRun.slice(0, 8);

// This looks like a backfill ID. Link there.
if (truncated === firstRun) {
return (
<Link to={`/runs/b/${firstRun}`}>
<Mono>{firstRun}</Mono>
</Link>
);
}

return (
<Box flex={{direction: 'row', gap: 4}}>
<Mono>{runIds[0]}</Mono>
</Box>
<Link to={`/runs/${firstRun}`}>
<Mono>{truncated}</Mono>
</Link>
hellendag marked this conversation as resolved.
Show resolved Hide resolved
);
}

Expand Down
Loading
Loading