diff --git a/js_modules/dagit/packages/core/src/app/Flags.tsx b/js_modules/dagit/packages/core/src/app/Flags.tsx index d378b53857677..5a2ab2decaed5 100644 --- a/js_modules/dagit/packages/core/src/app/Flags.tsx +++ b/js_modules/dagit/packages/core/src/app/Flags.tsx @@ -6,7 +6,6 @@ import {getJSONForKey} from '../hooks/useStateWithStorage'; export const DAGIT_FLAGS_KEY = 'DAGIT_FLAGS'; export enum FeatureFlag { - flagExperimentalAssetDAG = 'flagExperimentalAssetDAG', flagDebugConsoleLogging = 'flagDebugConsoleLogging', flagAlwaysCollapseNavigation = 'flagAlwaysCollapseNavigation', flagDisableWebsockets = 'flagDisableWebsockets', diff --git a/js_modules/dagit/packages/core/src/app/UserSettingsRoot.tsx b/js_modules/dagit/packages/core/src/app/UserSettingsRoot.tsx index ea587b0af1cbe..6a084d590844b 100644 --- a/js_modules/dagit/packages/core/src/app/UserSettingsRoot.tsx +++ b/js_modules/dagit/packages/core/src/app/UserSettingsRoot.tsx @@ -124,16 +124,6 @@ const UserSettingsRoot: React.FC = ({tabs}) => { /> ), }, - { - key: 'Experimental asset graph display', - value: ( - toggleFlag(FeatureFlag.flagExperimentalAssetDAG)} - /> - ), - }, { key: 'New partitions view (experimental)', value: ( 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 7a9ab42968453..632b0120e86e2 100644 --- a/js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx +++ b/js_modules/dagit/packages/core/src/asset-graph/AssetGraphExplorer.tsx @@ -1,13 +1,4 @@ -import { - Body, - Box, - Checkbox, - Colors, - Icon, - Mono, - NonIdealState, - SplitPanelContainer, -} from '@dagster-io/ui'; +import {Box, Checkbox, Colors, NonIdealState, SplitPanelContainer} from '@dagster-io/ui'; import flatMap from 'lodash/flatMap'; import isEqual from 'lodash/isEqual'; import pickBy from 'lodash/pickBy'; @@ -18,7 +9,6 @@ import React from 'react'; import {useHistory} from 'react-router-dom'; import styled from 'styled-components/macro'; -import {useFeatureFlags} from '../app/Flags'; import {GraphQueryItem} from '../app/GraphQueryImpl'; import { FIFTEEN_SECONDS, @@ -168,7 +158,6 @@ const AssetGraphExplorerWithData: React.FC< const history = useHistory(); const findJobForAsset = useFindJobForAsset(); - const {flagExperimentalAssetDAG: experiments} = useFeatureFlags(); const [highlighted, setHighlighted] = React.useState(null); @@ -344,99 +333,6 @@ const AssetGraphExplorerWithData: React.FC< - {Object.values(layout.bundles) - .sort((a, b) => a.id.length - b.id.length) - .map(({id, bounds}) => { - if (experiments && _scale < EXPERIMENTAL_MINI_SCALE) { - const path = JSON.parse(id); - return ( - { - viewportEl.current?.zoomToSVGBox(bounds, true, 1.2); - e.stopPropagation(); - }} - > - - hasPathPrefix(g.assetKey.path, path), - )} - > - -
- - {withMiddleTruncation(displayNameForAssetKey({path}), { - maxLength: 17, - })} -
- - {layout.bundleMapping[id].length} items - - - - - ); - } - return ( - - EXPERIMENTAL_MINI_SCALE - ? (_scale - EXPERIMENTAL_MINI_SCALE) / 0.2 - : 0, - fontWeight: 600, - display: 'flex', - gap: 6, - }} - > - - {displayNameForAssetKey({path: JSON.parse(id)})} - -
- - ); - })} - {Object.values(layout.nodes).map(({id, bounds}, index) => { const graphNode = assetGraphData.nodes[id]; const path = JSON.parse(id); @@ -451,15 +347,6 @@ const AssetGraphExplorerWithData: React.FC< ) : null; } - if (experiments) { - const isWithinBundle = Object.keys(layout.bundles).some((bundleId) => - hasPathPrefix(path, JSON.parse(bundleId)), - ); - if (isWithinBundle && _scale < EXPERIMENTAL_MINI_SCALE) { - return null; - } - } - return ( {!graphNode || !graphNode.definition.opNames.length ? ( - ) : experiments && _scale < EXPERIMENTAL_MINI_SCALE ? ( + ) : _scale < EXPERIMENTAL_MINI_SCALE ? ( return node.jobNames.length === 0 && !node.opNames.length; }; -export function identifyBundles(assetIds: string[]) { - const pathPrefixes: {[prefixId: string]: string[]} = {}; - - for (const assetId of assetIds) { - const assetKeyPath = JSON.parse(assetId); - - for (let ii = 1; ii < assetKeyPath.length; ii++) { - const prefix = assetKeyPath.slice(0, ii); - const key = JSON.stringify(prefix); - pathPrefixes[key] = pathPrefixes[key] || []; - pathPrefixes[key].push(assetId); - } - } - - for (const key of Object.keys(pathPrefixes)) { - if (pathPrefixes[key].length <= 1) { - delete pathPrefixes[key]; - } - } - - const finalBundlePrefixes: {[prefixId: string]: string[]} = {}; - const finalBundleIdForNodeId: {[id: string]: string} = {}; - - // Sort the prefix keys by length descending and iterate from the deepest folders first. - // Dedupe asset keys and replace asset keys we've already seen with the (deeper) folder - // they are within. This gets us "multi layer folders" of nodes. - - // Turn this: - // { - // "s3": [["s3", "collect"], ["s3", "prod", "a"], ["s3", "prod", "b"]], - // "s3/prod": ["s3", "prod", "a"], ["s3", "prod", "b"] - // } - - // Into this: - // { - // "s3/prod": ["s3", "prod", "a"], ["s3", "prod", "b"] - // "s3": [["s3", "collect"], ["s3", "prod"]], - // } - - for (const prefixId of Object.keys(pathPrefixes).sort((a, b) => b.length - a.length)) { - const contents = uniq( - pathPrefixes[prefixId].map((p) => - finalBundleIdForNodeId[p] ? finalBundleIdForNodeId[p] : p, - ), - ); - if (contents.length === 1 && finalBundlePrefixes[contents[0]]) { - // If this bundle contains exactly one bundle, no need to show both outlines. - // Just show the inner one. eg: a > b > asset1, a > b > asset2, just show a > b. - continue; - } - finalBundlePrefixes[prefixId] = contents; - finalBundlePrefixes[prefixId].forEach((id) => (finalBundleIdForNodeId[id] = prefixId)); - } - return finalBundlePrefixes; -} - export const buildGraphData = (assetNodes: AssetNode[]) => { const data: GraphData = { nodes: {}, diff --git a/js_modules/dagit/packages/core/src/asset-graph/layout.ts b/js_modules/dagit/packages/core/src/asset-graph/layout.ts index 01d1fe50584ae..2600c207a56d9 100644 --- a/js_modules/dagit/packages/core/src/asset-graph/layout.ts +++ b/js_modules/dagit/packages/core/src/asset-graph/layout.ts @@ -2,7 +2,7 @@ import * as dagre from 'dagre'; import {IBounds, IPoint} from '../graph/common'; -import {GraphData, GraphNode, GraphId, displayNameForAssetKey, identifyBundles} from './Utils'; +import {GraphData, GraphNode, GraphId, displayNameForAssetKey} from './Utils'; export interface AssetLayout { id: GraphId; @@ -23,12 +23,6 @@ export type AssetGraphLayout = { height: number; edges: AssetLayoutEdge[]; nodes: {[id: string]: AssetLayout}; - - bundles: {[id: string]: AssetLayout}; - bundleEdges: AssetLayoutEdge[]; - bundleMapping: { - [prefixId: string]: string[]; - }; }; const opts: {margin: number; mini: boolean} = { @@ -88,19 +82,6 @@ export const layoutAssetGraph = (graphData: GraphData): AssetGraphLayout => { g.setNode(id, getForeignNodeDimensions(id)); }); - // Create "parent" nodes for nodes with a shared ID (path) prefix (eg: s3>a, s3>b), - // and then place the children inside. Note that the bundles are identified in order - // and bundleMapping can reference bundles as children - this code can create multiple - // layers of parents! - const bundleMapping = identifyBundles(g.nodes()); - - for (const [parentId, nodeIds] of Object.entries(bundleMapping)) { - g.setNode(parentId, {}); - for (const nodeId of nodeIds) { - g.setParent(nodeId, parentId); - } - } - dagre.layout(g); const dagreNodesById: {[id: string]: dagre.Node} = {}; @@ -116,7 +97,6 @@ export const layoutAssetGraph = (graphData: GraphData): AssetGraphLayout => { let maxHeight = 0; const nodes: {[id: string]: AssetLayout} = {}; - const bundles: {[id: string]: AssetLayout} = {}; Object.keys(dagreNodesById).forEach((id) => { const dagreNode = dagreNodesById[id]; @@ -126,58 +106,27 @@ export const layoutAssetGraph = (graphData: GraphData): AssetGraphLayout => { width: dagreNode.width, height: dagreNode.height, }; - if (bundleMapping[id]) { - return; - } nodes[id] = {id, bounds}; - // If this node was inside one or more parent nodes, upsert the parent box - // into the bundles result set and expand it to include the child. Note: - // dagre does give us "parent" node dimensions, but sometimes they're randomly - // much larger than the contents. - let bundleId = g.parent(id); - while (bundleId) { - bundles[bundleId] = bundles[bundleId] || {id: bundleId, bounds}; - bundles[bundleId].bounds = extendBounds(bundles[bundleId].bounds, { - x: bounds.x - opts.margin / 4, - y: bounds.y - opts.margin / 4, - width: bounds.width + opts.margin / 2, - height: bounds.height + opts.margin / 2, - }); - bundleId = g.parent(bundleId); - } maxWidth = Math.max(maxWidth, dagreNode.x + dagreNode.width / 2); maxHeight = Math.max(maxHeight, dagreNode.y + dagreNode.height / 2); }); const edges: AssetLayoutEdge[] = []; - const bundleEdges: AssetLayoutEdge[] = []; g.edges().forEach((e) => { const points = g.edge(e).points; - if (bundles[e.v] || bundles[e.w]) { - bundleEdges.push({ - from: points[0], - fromId: e.v, - to: points[points.length - 1], - toId: e.w, - }); - } else { - edges.push({ - from: points[0], - fromId: e.v, - to: points[points.length - 1], - toId: e.w, - }); - } + edges.push({ + from: points[0], + fromId: e.v, + to: points[points.length - 1], + toId: e.w, + }); }); return { nodes, edges, - bundles, - bundleEdges, - bundleMapping, width: maxWidth + opts.margin, height: maxHeight + opts.margin, };