Skip to content

Commit

Permalink
[dagit] Remove the global asset graph in preparation for asset group …
Browse files Browse the repository at this point in the history
…graphs (#8125)

* [dagit] Add last materialization, latest run columns to the asset table

* [dagit] Remove the Asset Grid prototype that was behind a feature flag (#8109)

Co-authored-by: bengotow <bgotow@elementl.com>

* [dagit] Remove the global asset graph in preparation for asset group graphs

Co-authored-by: bengotow <bgotow@elementl.com>
  • Loading branch information
bengotow and bengotow committed May 31, 2022
1 parent 8f1d403 commit cc7360a
Show file tree
Hide file tree
Showing 11 changed files with 13 additions and 172 deletions.
4 changes: 2 additions & 2 deletions js_modules/dagit/packages/core/src/app/FallthroughRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ const FinalRedirectOrLoadingRoot = () => {

const reposWithAJob = allRepos.filter((r) => r.repository.pipelines.length > 0);

// If every loaded repo only contains asset jobs, route to the asset graph
// If every loaded repo only contains asset jobs, route to the asset catalog
if (
reposWithAJob.every(({repository}) => repository.pipelines.every((p) => isAssetGroup(p.name)))
) {
return <Redirect to="/instance/asset-graph" />;
return <Redirect to="/instance/assets" />;
}

// If we have exactly one repo with one job, route to the job overview
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
LargeDAGNotice,
LoadingNotice,
} from '../pipelines/GraphNotices';
import {ExplorerPath, instanceAssetsExplorerPathToURL} from '../pipelines/PipelinePathUtils';
import {ExplorerPath} from '../pipelines/PipelinePathUtils';
import {SidebarPipelineOrJobOverview} from '../pipelines/SidebarPipelineOrJobOverview';
import {useDidLaunchEvent} from '../runs/RunUtils';
import {PipelineSelector} from '../types/globalTypes';
Expand Down Expand Up @@ -181,8 +181,6 @@ const AssetGraphExplorerWithData: React.FC<
? selectedGraphNodes
: Object.values(assetGraphData.nodes).filter((a) => !isSourceAsset(a.definition));

const isGlobalGraph = !pipelineSelector;

const onSelectNode = React.useCallback(
async (
e: React.MouseEvent<any> | React.KeyboardEvent<any>,
Expand All @@ -205,29 +203,17 @@ const AssetGraphExplorerWithData: React.FC<
clicked = await findJobForAsset(assetKey);
}

if (!clicked.opNames.length) {
if (!clicked.jobName || !clicked.opNames.length) {
// This op has no definition in any loaded repository (source asset).
// The best we can do is show the asset page. This will still be mostly empty,
// but there can be a description.
history.push(`/instance/assets/${assetKey.path.join('/')}?view=definition`);
return;
}

if (!clicked.jobName && !isGlobalGraph) {
// This asset has a definition (opName) but isn't in any non asset-group jobs.
// We can switch to the instance-wide asset graph and see it in context there.
history.push(
instanceAssetsExplorerPathToURL({
opsQuery: `++"${token}"++`,
opNames: [token],
}),
);
return;
}

// This asset is in different job (and we're in the job explorer),
// go to the other job.
if (!isGlobalGraph && clicked.jobName !== explorerPath.pipelineName) {
if (clicked.jobName !== explorerPath.pipelineName) {
onChangeExplorerPath(
{...explorerPath, opNames: [token], opsQuery: '', pipelineName: clicked.jobName!},
'replace',
Expand Down Expand Up @@ -265,7 +251,6 @@ const AssetGraphExplorerWithData: React.FC<
);
},
[
isGlobalGraph,
explorerPath,
onChangeExplorerPath,
findJobForAsset,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {gql} from '@apollo/client';
import {Colors, Icon, Spinner, Tooltip, FontFamily, Box, CaptionMono} from '@dagster-io/ui';
import {Colors, Icon, Tooltip, FontFamily, Box, CaptionMono, Spinner} from '@dagster-io/ui';
import isEqual from 'lodash/isEqual';
import React, {CSSProperties} from 'react';
import {Link} from 'react-router-dom';
Expand Down
35 changes: 0 additions & 35 deletions js_modules/dagit/packages/core/src/assets/AssetNodeDefinition.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ import {
displayNameForAssetKey,
isSourceAsset,
LiveData,
tokenForAssetKey,
isAssetGroup,
__ASSET_GROUP_PREFIX,
} from '../asset-graph/Utils';
import {DagsterTypeSummary} from '../dagstertype/DagsterType';
import {Description} from '../pipelines/Description';
import {instanceAssetsExplorerPathToURL} from '../pipelines/PipelinePathUtils';
import {PipelineReference} from '../pipelines/PipelineReference';
import {buildRepoAddress} from '../workspace/buildRepoAddress';
import {RepoAddress} from '../workspace/types';
Expand Down Expand Up @@ -89,7 +87,6 @@ export const AssetNodeDefinition: React.FC<{
flex={{justifyContent: 'space-between', gap: 8}}
>
<Subheading>Upstream Assets ({assetNode.dependencies.length})</Subheading>
<JobGraphLink repoAddress={repoAddress} assetNode={assetNode} direction="upstream" />
</Box>
<AssetNodeList items={assetNode.dependencies} liveDataByNode={liveDataByNode} />

Expand All @@ -99,7 +96,6 @@ export const AssetNodeDefinition: React.FC<{
flex={{justifyContent: 'space-between', gap: 8}}
>
<Subheading>Downstream Assets ({assetNode.dependedBy.length})</Subheading>
<JobGraphLink repoAddress={repoAddress} assetNode={assetNode} direction="downstream" />
</Box>
<AssetNodeList items={assetNode.dependedBy} liveDataByNode={liveDataByNode} />
</Box>
Expand Down Expand Up @@ -137,37 +133,6 @@ export const AssetNodeDefinition: React.FC<{
);
};

const JobGraphLink: React.FC<{
repoAddress: RepoAddress;
assetNode: AssetNodeDefinitionFragment;
direction: 'upstream' | 'downstream';
}> = ({direction, assetNode}) => {
if (isSourceAsset(assetNode)) {
return null;
}
const populated =
(direction === 'upstream' ? assetNode.dependencies : assetNode.dependedBy).length > 0;
if (!populated) {
return null;
}

const token = tokenForAssetKey(assetNode.assetKey);

return (
<Link
to={instanceAssetsExplorerPathToURL({
opNames: [],
opsQuery: direction === 'upstream' ? `*"${token}"` : `"${token}"*`,
})}
>
<Box flex={{gap: 4, alignItems: 'center'}}>
{direction === 'upstream' ? 'View upstream graph' : 'View downstream graph'}
<Icon name="open_in_new" color={Colors.Link} />
</Box>
</Link>
);
};

const DefinitionLocation: React.FC<{
assetNode: AssetNodeDefinitionFragment;
repoAddress: RepoAddress;
Expand Down
28 changes: 4 additions & 24 deletions js_modules/dagit/packages/core/src/assets/AssetTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ import styled from 'styled-components/macro';

import {usePermissions} from '../app/Permissions';
import {AssetLatestRunWithNotices, AssetRunLink} from '../asset-graph/AssetNode';
import {LiveData, toGraphId, tokenForAssetKey} from '../asset-graph/Utils';
import {LiveData, toGraphId} from '../asset-graph/Utils';
import {useSelectionReducer} from '../hooks/useSelectionReducer';
import {RepositoryLink} from '../nav/RepositoryLink';
import {instanceAssetsExplorerPathToURL} from '../pipelines/PipelinePathUtils';
import {TimestampDisplay} from '../schedules/TimestampDisplay';
import {MenuLink} from '../ui/MenuLink';
import {markdownToPlaintext} from '../ui/markdownToPlaintext';
Expand Down Expand Up @@ -164,7 +163,6 @@ const AssetEntryRow: React.FC<{
const linkUrl = `/instance/assets/${fullPath.map(encodeURIComponent).join('/')}`;

const isGroup = assets.length > 1 || fullPath.join('/') !== assets[0].key.path.join('/');
const representsAtLeastOneSDA = assets.some((a) => !!a.definition);
const asset = !isGroup ? assets[0] : null;

const onChange = (e: React.FormEvent<HTMLInputElement>) => {
Expand Down Expand Up @@ -239,18 +237,9 @@ const AssetEntryRow: React.FC<{
<td>
{asset ? (
<Box flex={{gap: 8, alignItems: 'center'}}>
{asset.definition?.opNames.length ? (
<Link
to={instanceAssetsExplorerPathToURL({
opsQuery: `++"${tokenForAssetKey({path})}"++`,
opNames: [tokenForAssetKey({path})],
})}
>
<Button>View in Asset Graph</Button>
</Link>
) : (
<Button disabled={true}>View in Asset Graph</Button>
)}
<Link to={`/instance/assets/${path.join('/')}`}>
<Button>View Details</Button>
</Link>
<Popover
position="bottom-right"
content={
Expand All @@ -273,15 +262,6 @@ const AssetEntryRow: React.FC<{
<Button icon={<Icon name="expand_more" />} />
</Popover>
</Box>
) : representsAtLeastOneSDA ? (
<Link
to={instanceAssetsExplorerPathToURL({
opsQuery: `${tokenForAssetKey({path})}/`,
opNames: [],
})}
>
<Button>View in Asset Graph</Button>
</Link>
) : (
<span />
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,15 @@
import {ButtonGroup, ButtonGroupItem} from '@dagster-io/ui';
import * as React from 'react';
import {useHistory} from 'react-router-dom';

import {AssetViewType, useAssetView} from './useAssetView';

export const AssetViewModeSwitch = () => {
const history = useHistory();
const [view, _setView] = useAssetView();
const [view, setView] = useAssetView();

const buttons: ButtonGroupItem<AssetViewType>[] = [
{id: 'graph', icon: 'gantt_waterfall', tooltip: 'Graph view'},
{id: 'flat', icon: 'view_list', tooltip: 'List view'},
{id: 'directory', icon: 'folder', tooltip: 'Folder view'},
];

const setView = (view: AssetViewType) => {
_setView(view);
if (view === 'graph') {
history.push('/instance/asset-graph');
} else if (history.location.pathname !== '/instance/assets') {
history.push('/instance/assets');
}
};

return <ButtonGroup activeItems={new Set([view])} buttons={buttons} onClick={setView} />;
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {gql, useQuery} from '@apollo/client';
import * as React from 'react';
import {Redirect} from 'react-router-dom';
import styled from 'styled-components/macro';

import {Box, CursorPaginationControls, CursorPaginationProps, TextInput} from '../../../ui/src';
Expand Down Expand Up @@ -43,7 +42,6 @@ export const AssetsCatalogTable: React.FC<{prefixPath?: string[]}> = ({prefixPat

const assetsQuery = useQuery<AssetCatalogTableQuery>(ASSET_CATALOG_TABLE_QUERY, {
notifyOnNetworkStatusChange: true,
skip: view === 'graph',
});

const assetsOrError = assetsQuery.data?.assetsOrError;
Expand Down Expand Up @@ -99,10 +97,6 @@ export const AssetsCatalogTable: React.FC<{prefixPath?: string[]}> = ({prefixPat
}
}, [view, _setView, prefixPath]);

if (view === 'graph' && !prefixPath.length) {
return <Redirect to="/instance/asset-graph" />;
}

if (assetsOrError?.__typename === 'PythonError') {
return <PythonErrorInfo error={assetsOrError} />;
}
Expand Down

This file was deleted.

9 changes: 2 additions & 7 deletions js_modules/dagit/packages/core/src/assets/useAssetView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,10 @@ import {useStateWithStorage} from '../hooks/useStateWithStorage';

const ASSET_VIEW_KEY = 'AssetViewPreference';

export type AssetViewType = 'flat' | 'directory' | 'graph' | 'grid';
export type AssetViewType = 'flat' | 'directory';

const validateSavedAssetView = (storedValue: any) =>
storedValue === 'flat' ||
storedValue === 'directory' ||
storedValue === 'graph' ||
storedValue === 'grid'
? storedValue
: 'flat';
storedValue === 'flat' || storedValue === 'directory' ? storedValue : 'flat';

export const useAssetView = () => {
return useStateWithStorage<AssetViewType>(ASSET_VIEW_KEY, validateSavedAssetView);
Expand Down
4 changes: 0 additions & 4 deletions js_modules/dagit/packages/core/src/instance/InstanceRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {Redirect, Route, Switch, useLocation} from 'react-router-dom';

import {AssetEntryRoot} from '../assets/AssetEntryRoot';
import {AssetsCatalogRoot} from '../assets/AssetsCatalogRoot';
import {InstanceAssetGraphExplorer} from '../assets/InstanceAssetGraphExplorer';
import {RunRoot} from '../runs/RunRoot';
import {RunsRoot} from '../runs/RunsRoot';
import {SnapshotRoot} from '../snapshots/SnapshotRoot';
Expand All @@ -25,9 +24,6 @@ export const InstanceRoot = () => {
<Route path="/instance/assets" exact>
<AssetsCatalogRoot />
</Route>
<Route path="/instance/asset-graph(/?.*)">
<InstanceAssetGraphExplorer />
</Route>
<Route path="/instance/assets/(/?.*)">
<AssetEntryRoot />
</Route>
Expand Down
16 changes: 0 additions & 16 deletions js_modules/dagit/packages/core/src/pipelines/PipelinePathUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,6 @@ export function explorerPathFromString(path: string): ExplorerPath {
};
}

export function instanceAssetsExplorerPathFromString(path: string): ExplorerPath {
// This is a bit of a hack, but our explorer path needs a job name and we'd like
// to continue sharing the parsing/stringifying logic from the job graph UI
return explorerPathFromString(__ASSET_GROUP_PREFIX + path || '/');
}

export function instanceAssetsExplorerPathToURL(path: Omit<ExplorerPath, 'pipelineName'>) {
return (
'/instance/asset-graph' +
explorerPathToString({...path, pipelineName: __ASSET_GROUP_PREFIX}).replace(
__ASSET_GROUP_PREFIX,
'',
)
);
}

export function useStripSnapshotFromPath(params: {pipelinePath: string}) {
const history = useHistory();
const {pipelinePath} = params;
Expand Down

0 comments on commit cc7360a

Please sign in to comment.