From 1b999747990703c4841ceea9973f8e5ca7af018f Mon Sep 17 00:00:00 2001 From: bengotow Date: Tue, 31 May 2022 11:57:50 -0500 Subject: [PATCH] [dagit] Remove the global asset graph in preparation for asset group graphs --- .../packages/core/src/app/FallthroughRoot.tsx | 4 +- .../src/asset-graph/AssetGraphExplorer.tsx | 21 ++------- .../core/src/assets/AssetNodeDefinition.tsx | 35 -------------- .../packages/core/src/assets/AssetTable.tsx | 25 ++-------- .../core/src/assets/AssetViewModeSwitch.tsx | 14 +----- .../core/src/assets/AssetsCatalogTable.tsx | 5 -- .../src/assets/InstanceAssetGraphExplorer.tsx | 46 ------------------- .../core/src/instance/InstanceRoot.tsx | 4 -- .../core/src/pipelines/PipelinePathUtils.tsx | 16 ------- 9 files changed, 9 insertions(+), 161 deletions(-) delete mode 100644 js_modules/dagit/packages/core/src/assets/InstanceAssetGraphExplorer.tsx diff --git a/js_modules/dagit/packages/core/src/app/FallthroughRoot.tsx b/js_modules/dagit/packages/core/src/app/FallthroughRoot.tsx index 6b6203d420812..fe5baef959948 100644 --- a/js_modules/dagit/packages/core/src/app/FallthroughRoot.tsx +++ b/js_modules/dagit/packages/core/src/app/FallthroughRoot.tsx @@ -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 ; + return ; } // If we have exactly one repo with one job, route to the job overview diff --git a/js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx b/js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx index b8d7f6a2129fe..7a9ab42968453 100644 --- a/js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx +++ b/js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx @@ -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'; @@ -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 | React.KeyboardEvent, @@ -205,7 +203,7 @@ 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. @@ -213,21 +211,9 @@ const AssetGraphExplorerWithData: React.FC< 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', @@ -265,7 +251,6 @@ const AssetGraphExplorerWithData: React.FC< ); }, [ - isGlobalGraph, explorerPath, onChangeExplorerPath, findJobForAsset, diff --git a/js_modules/dagit/packages/core/src/assets/AssetNodeDefinition.tsx b/js_modules/dagit/packages/core/src/assets/AssetNodeDefinition.tsx index fff139ee61e89..58add294bcc77 100644 --- a/js_modules/dagit/packages/core/src/assets/AssetNodeDefinition.tsx +++ b/js_modules/dagit/packages/core/src/assets/AssetNodeDefinition.tsx @@ -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'; @@ -89,7 +87,6 @@ export const AssetNodeDefinition: React.FC<{ flex={{justifyContent: 'space-between', gap: 8}} > Upstream Assets ({assetNode.dependencies.length}) - @@ -99,7 +96,6 @@ export const AssetNodeDefinition: React.FC<{ flex={{justifyContent: 'space-between', gap: 8}} > Downstream Assets ({assetNode.dependedBy.length}) - @@ -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 ( - - - {direction === 'upstream' ? 'View upstream graph' : 'View downstream graph'} - - - - ); -}; - const DefinitionLocation: React.FC<{ assetNode: AssetNodeDefinitionFragment; repoAddress: RepoAddress; diff --git a/js_modules/dagit/packages/core/src/assets/AssetTable.tsx b/js_modules/dagit/packages/core/src/assets/AssetTable.tsx index 159b8fcab5e0d..54fce37c66a5b 100644 --- a/js_modules/dagit/packages/core/src/assets/AssetTable.tsx +++ b/js_modules/dagit/packages/core/src/assets/AssetTable.tsx @@ -20,7 +20,6 @@ import {AssetLatestRunWithNotices, AssetRunLink} from '../asset-graph/AssetNode' import {LiveData, toGraphId, tokenForAssetKey} 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'; @@ -239,18 +238,9 @@ const AssetEntryRow: React.FC<{ {asset ? ( - {asset.definition?.opNames.length ? ( - - - - ) : ( - - )} + + + } /> - ) : representsAtLeastOneSDA ? ( - - - ) : ( )} diff --git a/js_modules/dagit/packages/core/src/assets/AssetViewModeSwitch.tsx b/js_modules/dagit/packages/core/src/assets/AssetViewModeSwitch.tsx index 85ad99870870a..ea5922f2227f4 100644 --- a/js_modules/dagit/packages/core/src/assets/AssetViewModeSwitch.tsx +++ b/js_modules/dagit/packages/core/src/assets/AssetViewModeSwitch.tsx @@ -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[] = [ - {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 ; }; diff --git a/js_modules/dagit/packages/core/src/assets/AssetsCatalogTable.tsx b/js_modules/dagit/packages/core/src/assets/AssetsCatalogTable.tsx index 02e65a6569d81..cc39b962365c2 100644 --- a/js_modules/dagit/packages/core/src/assets/AssetsCatalogTable.tsx +++ b/js_modules/dagit/packages/core/src/assets/AssetsCatalogTable.tsx @@ -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'; @@ -99,10 +98,6 @@ export const AssetsCatalogTable: React.FC<{prefixPath?: string[]}> = ({prefixPat } }, [view, _setView, prefixPath]); - if (view === 'graph' && !prefixPath.length) { - return ; - } - if (assetsOrError?.__typename === 'PythonError') { return ; } diff --git a/js_modules/dagit/packages/core/src/assets/InstanceAssetGraphExplorer.tsx b/js_modules/dagit/packages/core/src/assets/InstanceAssetGraphExplorer.tsx deleted file mode 100644 index ac13499ba8ebd..0000000000000 --- a/js_modules/dagit/packages/core/src/assets/InstanceAssetGraphExplorer.tsx +++ /dev/null @@ -1,46 +0,0 @@ -import {Box, Colors, Heading, PageHeader} from '@dagster-io/ui'; -import * as React from 'react'; -import {useParams} from 'react-router'; -import {useHistory} from 'react-router-dom'; - -import {AssetGraphExplorer} from '../asset-graph/AssetGraphExplorer'; -import { - instanceAssetsExplorerPathFromString, - instanceAssetsExplorerPathToURL, -} from '../pipelines/PipelinePathUtils'; -import {ReloadAllButton} from '../workspace/ReloadAllButton'; - -import {AssetViewModeSwitch} from './AssetViewModeSwitch'; - -export const InstanceAssetGraphExplorer: React.FC = () => { - const params = useParams(); - const history = useHistory(); - const explorerPath = instanceAssetsExplorerPathFromString(params[0]); - - return ( - - Assets} - right={} - /> - - - - { - history[mode](instanceAssetsExplorerPathToURL(path)); - }} - /> - - ); -}; diff --git a/js_modules/dagit/packages/core/src/instance/InstanceRoot.tsx b/js_modules/dagit/packages/core/src/instance/InstanceRoot.tsx index 54b3fa6c1ce3f..183a96a4a233e 100644 --- a/js_modules/dagit/packages/core/src/instance/InstanceRoot.tsx +++ b/js_modules/dagit/packages/core/src/instance/InstanceRoot.tsx @@ -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'; @@ -25,9 +24,6 @@ export const InstanceRoot = () => { - - - diff --git a/js_modules/dagit/packages/core/src/pipelines/PipelinePathUtils.tsx b/js_modules/dagit/packages/core/src/pipelines/PipelinePathUtils.tsx index 8ee94722b2194..4c73ea1252e4d 100644 --- a/js_modules/dagit/packages/core/src/pipelines/PipelinePathUtils.tsx +++ b/js_modules/dagit/packages/core/src/pipelines/PipelinePathUtils.tsx @@ -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) { - 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;