diff --git a/src/common/formatters.ts b/src/common/formatters.ts index ce94cab7e..0011c0459 100644 --- a/src/common/formatters.ts +++ b/src/common/formatters.ts @@ -144,3 +144,12 @@ export function ensureUrlWithProtocol(url: string): string { } return url; } + +/** Formats a number into a string with leading zeros to ensure a consistent + * width. + * Example: 1 will be '01' + * 10 will be '10' + */ +export function leftPaddedNumber(value: number, length: number): string { + return value.toString().padStart(length, '0'); +} diff --git a/src/common/test/formatters.spec.ts b/src/common/test/formatters.spec.ts index 2e82700d7..718ce5db0 100644 --- a/src/common/test/formatters.spec.ts +++ b/src/common/test/formatters.spec.ts @@ -1,5 +1,4 @@ import { millisecondsToDuration } from 'common/utils'; -import { Admin } from 'flyteidl'; import { subSecondString, unknownValueString, @@ -10,10 +9,10 @@ import { dateFromNow, dateWithFromNow, ensureUrlWithProtocol, - fixedRateToString, formatDate, formatDateLocalTimezone, formatDateUTC, + leftPaddedNumber, millisecondsToHMS, protobufDurationToHMS } from '../formatters'; @@ -177,3 +176,24 @@ describe('ensureUrlWithProtocol', () => { }) ); }); + +describe('leftPaddedNumber', () => { + const cases: [number, number, string][] = [ + [1, 0, '1'], + [10, 0, '10'], + [0, 1, '0'], + [0, 2, '00'], + [1, 1, '1'], + [1, 2, '01'], + [1, 3, '001'], + [10, 1, '10'], + [10, 2, '10'], + [10, 3, '010'] + ]; + + cases.forEach(([value, width, expected]) => + it(`should produce ${expected} with input (${value}, ${width})`, () => { + expect(leftPaddedNumber(value, width)).toEqual(expected); + }) + ); +}); diff --git a/src/components/Cache/createCache.ts b/src/components/Cache/createCache.ts index 9886d30ef..dfd12d9ec 100644 --- a/src/components/Cache/createCache.ts +++ b/src/components/Cache/createCache.ts @@ -14,9 +14,9 @@ function hasId(value: Object): value is HasIdObject { * strictly equal (===). The cache provides methods for getting/setting values * by key and for merging values or arrays of values with any existing items. */ -export interface ValueCache { +export interface ValueCache { /** Retrieve an item by key */ - get(key: EntityKey): object | undefined; + get(key: EntityKey): ValueType | undefined; /** Check existence of an item by key */ has(key: EntityKey): boolean; /** Merge an array of values. If the items have an `id` property, its value @@ -30,9 +30,9 @@ export interface ValueCache { * performed. For arrays, the value is _replaced_. * @returns The merged value */ - mergeValue(key: EntityKey, value: object): object; + mergeValue(key: EntityKey, value: ValueType): ValueType; /** Set an item value by key. Replaces any existing value. */ - set(key: EntityKey, value: object): object; + set(key: EntityKey, value: ValueType): ValueType; } type Cacheable = object | any[]; diff --git a/src/components/Cache/utils.ts b/src/components/Cache/utils.ts index 40c1f6a74..c55401218 100644 --- a/src/components/Cache/utils.ts +++ b/src/components/Cache/utils.ts @@ -6,5 +6,6 @@ import * as objectHash from 'object-hash'; export function getCacheKey(id: any[] | object | string) { return typeof id === 'string' || typeof id === 'symbol' ? id - : objectHash(id); + : // We only want to compare own properties, not .__proto__, .constructor, etc. + objectHash(id, { respectType: false }); } diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx index 170afaf7d..ecf511921 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx @@ -1,14 +1,12 @@ import { WaitForData, withRouteParams } from 'components/common'; -import { - RefreshConfig, - useDataRefresher, - useWorkflowExecution -} from 'components/hooks'; +import { RefreshConfig, useDataRefresher } from 'components/hooks'; import { Execution } from 'models'; import * as React from 'react'; import { executionRefreshIntervalMs } from '../constants'; +import { ExecutionContext, ExecutionDataCacheContext } from '../contexts'; +import { useExecutionDataCache } from '../useExecutionDataCache'; +import { useWorkflowExecution } from '../useWorkflowExecution'; import { executionIsTerminal } from '../utils'; -import { ExecutionContext } from './contexts'; import { ExecutionDetailsAppBarContent } from './ExecutionDetailsAppBarContent'; import { ExecutionNodeViews } from './ExecutionNodeViews'; @@ -35,15 +33,23 @@ export const ExecutionDetailsContainer: React.FC = domain: domainId, name: executionId }; - - const { fetchable, terminateExecution } = useWorkflowExecution(id); + const dataCache = useExecutionDataCache(); + const { fetchable, terminateExecution } = useWorkflowExecution( + id, + dataCache + ); useDataRefresher(id, fetchable, executionRefreshConfig); - const contextValue = { terminateExecution, execution: fetchable.value }; + const contextValue = { + terminateExecution, + execution: fetchable.value + }; return ( - + + + ); diff --git a/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/InputNodeDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/InputNodeDetails.tsx index 2ec0aa417..56c19d460 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/InputNodeDetails.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/InputNodeDetails.tsx @@ -6,7 +6,7 @@ import { NodeDetailsProps } from 'components/WorkflowGraph'; import { useStyles as useBaseStyles } from 'components/WorkflowGraph/NodeDetails/styles'; import { LiteralMapViewer } from 'components/Literals'; -import { ExecutionContext } from '../contexts'; +import { ExecutionContext } from '../../contexts'; /** Details panel renderer for the start/input node in a graph. Displays the * top level `WorkflowExecution` inputs. diff --git a/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/OutputNodeDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/OutputNodeDetails.tsx index 5afc25cb0..9d2b5530f 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/OutputNodeDetails.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/OutputNodeDetails.tsx @@ -1,12 +1,12 @@ import { SectionHeader, WaitForData } from 'components/common'; import { useCommonStyles } from 'components/common/styles'; -import { useWorkflowExecutionData } from 'components/hooks'; import { LiteralMapViewer, RemoteLiteralMapViewer } from 'components/Literals'; import { NodeDetailsProps } from 'components/WorkflowGraph'; import { useStyles as useBaseStyles } from 'components/WorkflowGraph/NodeDetails/styles'; import { emptyLiteralMapBlob, Execution } from 'models'; import * as React from 'react'; -import { ExecutionContext } from '../contexts'; +import { ExecutionContext } from '../../contexts'; +import { useWorkflowExecutionData } from '../../useWorkflowExecution'; const RemoteExecutionOutputs: React.FC<{ execution: Execution }> = ({ execution diff --git a/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/TaskExecutionNodeDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/TaskExecutionNodeDetails.tsx index 0bc4e8d20..c0dc40132 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/TaskExecutionNodeDetails.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionNodeDetails/TaskExecutionNodeDetails.tsx @@ -14,7 +14,7 @@ import { TaskNodeDetails } from 'components/WorkflowGraph'; import { useStyles as useBaseStyles } from 'components/WorkflowGraph/NodeDetails/styles'; -import { NodeExecutionsContext } from '../contexts'; +import { NodeExecutionsContext } from '../../contexts'; import { NodeExecutionData } from '../NodeExecutionData'; import { NodeExecutionInputs } from '../NodeExecutionInputs'; import { NodeExecutionOutputs } from '../NodeExecutionOutputs'; diff --git a/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx b/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx index 8e1c67ade..d5d39d4e4 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx @@ -4,6 +4,7 @@ import { WaitForData } from 'components/common'; import { useTabState } from 'components/hooks/useTabState'; import { Execution } from 'models'; import * as React from 'react'; +import { NodeExecutionsRequestConfigContext } from '../contexts'; import { ExecutionFilters } from '../ExecutionFilters'; import { useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState'; import { NodeExecutionsTable } from '../Tables/NodeExecutionsTable'; @@ -43,10 +44,11 @@ export const ExecutionNodeViews: React.FC = ({ const filterState = useNodeExecutionFiltersState(); const tabState = useTabState(tabIds, tabIds.nodes); - const { workflow, nodeExecutions } = useWorkflowExecutionState( - execution, - filterState.appliedFilters - ); + const { + workflow, + nodeExecutions, + nodeExecutionsRequestConfig + } = useWorkflowExecutionState(execution, filterState.appliedFilters); return ( @@ -61,10 +63,14 @@ export const ExecutionNodeViews: React.FC = ({ - + + + )} diff --git a/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx b/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx index e61046165..498c56b2b 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx @@ -1,16 +1,16 @@ import { DetailsPanel } from 'components/common'; import { WorkflowGraph } from 'components/WorkflowGraph'; import { keyBy } from 'lodash'; -import { endNodeId, startNodeId } from 'models'; +import { endNodeId, NodeExecution, startNodeId } from 'models'; import { Workflow } from 'models/Workflow'; import * as React from 'react'; -import { DetailedNodeExecution } from '../types'; -import { NodeExecutionsContext } from './contexts'; +import { NodeExecutionsContext } from '../contexts'; +import { useDetailedNodeExecutions } from '../useDetailedNodeExecutions'; import { NodeExecutionDetails } from './NodeExecutionDetails'; import { TaskExecutionNodeRenderer } from './TaskExecutionNodeRenderer/TaskExecutionNodeRenderer'; export interface ExecutionWorkflowGraphProps { - nodeExecutions: DetailedNodeExecution[]; + nodeExecutions: NodeExecution[]; workflow: Workflow; } @@ -19,9 +19,10 @@ export const ExecutionWorkflowGraph: React.FC = ({ nodeExecutions, workflow }) => { + const detailedNodeExecutions = useDetailedNodeExecutions(nodeExecutions); const nodeExecutionsById = React.useMemo( - () => keyBy(nodeExecutions, 'id.nodeId'), - [nodeExecutions] + () => keyBy(detailedNodeExecutions, 'id.nodeId'), + [detailedNodeExecutions] ); const [selectedNodes, setSelectedNodes] = React.useState([]); const onNodeSelectionChanged = (newSelection: string[]) => { diff --git a/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx index a933af4ad..b869ae77f 100644 --- a/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx +++ b/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx @@ -35,6 +35,9 @@ const useStyles = makeStyles((theme: Theme) => { content: { overflowY: 'auto' }, + displayId: { + marginBottom: theme.spacing(1) + }, header: { borderBottom: `${theme.spacing(1)}px solid ${theme.palette.divider}` }, @@ -60,8 +63,7 @@ const useStyles = makeStyles((theme: Theme) => { title: { alignItems: 'flex-start', display: 'flex', - justifyContent: 'space-between', - marginBottom: theme.spacing(1) + justifyContent: 'space-between' } }; }); @@ -189,6 +191,16 @@ export const NodeExecutionDetails: React.FC = ({ + + {execution.displayId} + {statusContent} diff --git a/src/components/Executions/ExecutionDetails/TaskExecutionNodeRenderer/TaskExecutionNode.tsx b/src/components/Executions/ExecutionDetails/TaskExecutionNodeRenderer/TaskExecutionNode.tsx index e7299f9a5..3f9e23b28 100644 --- a/src/components/Executions/ExecutionDetails/TaskExecutionNodeRenderer/TaskExecutionNode.tsx +++ b/src/components/Executions/ExecutionDetails/TaskExecutionNodeRenderer/TaskExecutionNode.tsx @@ -6,7 +6,7 @@ import { TaskNodeRenderer } from 'components/WorkflowGraph/TaskNodeRenderer'; import { NodeExecutionPhase } from 'models/Execution/enums'; import { DAGNode } from 'models/Graph'; -import { NodeExecutionsContext } from '../contexts'; +import { NodeExecutionsContext } from '../../contexts'; import { StatusIndicator } from './StatusIndicator'; /** Renders DAGNodes with colors based on their node type, as well as dots to diff --git a/src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx b/src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx new file mode 100644 index 000000000..7002576b0 --- /dev/null +++ b/src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx @@ -0,0 +1,45 @@ +import { render, waitFor } from '@testing-library/react'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext } from 'components/data/apiContext'; +import { + DetailedNodeExecution, + NodeExecutionDisplayType +} from 'components/Executions/types'; +import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData'; +import { listTaskExecutions } from 'models/Execution/api'; +import * as React from 'react'; +import { NodeExecutionDetails } from '../NodeExecutionDetails'; + +describe('NodeExecutionDetails', () => { + let execution: DetailedNodeExecution; + let mockListTaskExecutions: jest.Mock>; + beforeEach(() => { + const { executions } = createMockNodeExecutions(1); + execution = { + ...executions[0], + displayType: NodeExecutionDisplayType.PythonTask, + displayId: 'com.flyte.testTask', + cacheKey: 'abcdefg' + }; + mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] }); + }); + + const renderComponent = () => + render( + + + + ); + + it('renders displayId', async () => { + const { queryByText } = renderComponent(); + await waitFor(() => {}); + expect(queryByText(execution.displayId)).toBeInTheDocument(); + }); +}); diff --git a/src/components/Executions/ExecutionInputsOutputsModal.tsx b/src/components/Executions/ExecutionInputsOutputsModal.tsx index 6b6f4ac03..d33980efe 100644 --- a/src/components/Executions/ExecutionInputsOutputsModal.tsx +++ b/src/components/Executions/ExecutionInputsOutputsModal.tsx @@ -2,10 +2,10 @@ import { Dialog, DialogContent, Tab, Tabs } from '@material-ui/core'; import { makeStyles, Theme } from '@material-ui/core/styles'; import { WaitForData } from 'components/common'; import { ClosableDialogTitle } from 'components/common/ClosableDialogTitle'; -import { useWorkflowExecutionData } from 'components/hooks'; import { LiteralMapViewer, RemoteLiteralMapViewer } from 'components/Literals'; import { emptyLiteralMapBlob, Execution } from 'models'; import * as React from 'react'; +import { useWorkflowExecutionData } from './useWorkflowExecution'; const useStyles = makeStyles((theme: Theme) => ({ content: { diff --git a/src/components/Executions/Tables/NodeExecutionChildren.tsx b/src/components/Executions/Tables/NodeExecutionChildren.tsx index 8b9eda93c..ee3bb32f5 100644 --- a/src/components/Executions/Tables/NodeExecutionChildren.tsx +++ b/src/components/Executions/Tables/NodeExecutionChildren.tsx @@ -1,132 +1,67 @@ -import { WaitForData } from 'components/common'; -import { - RefreshConfig, - useDataRefresher, - useWorkflowExecution -} from 'components/hooks'; -import { isEqual } from 'lodash'; -import { Execution, NodeExecutionClosure, WorkflowNodeMetadata } from 'models'; +import { Typography } from '@material-ui/core'; +import * as classnames from 'classnames'; +import { useTheme } from 'components/Theme/useTheme'; import * as React from 'react'; -import { executionIsTerminal, executionRefreshIntervalMs } from '..'; -import { ExecutionContext } from '../ExecutionDetails/contexts'; -import { DetailedNodeExecution } from '../types'; -import { useDetailedTaskExecutions } from '../useDetailedTaskExecutions'; -import { - useTaskExecutions, - useTaskExecutionsRefresher -} from '../useTaskExecutions'; -import { generateRowSkeleton } from './generateRowSkeleton'; -import { NoExecutionsContent } from './NoExecutionsContent'; -import { useColumnStyles } from './styles'; -import { generateColumns as generateTaskExecutionColumns } from './taskExecutionColumns'; -import { TaskExecutionRow } from './TaskExecutionRow'; -import { generateColumns as generateWorkflowExecutionColumns } from './workflowExecutionColumns'; -import { WorkflowExecutionRow } from './WorkflowExecutionRow'; +import { DetailedNodeExecutionGroup } from '../types'; +import { NodeExecutionRow } from './NodeExecutionRow'; +import { useExecutionTableStyles } from './styles'; +import { calculateNodeExecutionRowLeftSpacing } from './utils'; export interface NodeExecutionChildrenProps { - execution: DetailedNodeExecution; + childGroups: DetailedNodeExecutionGroup[]; + level: number; } -const TaskNodeExecutionChildren: React.FC = ({ - execution: nodeExecution +/** Renders a nested list of row items for children of a NodeExecution */ +export const NodeExecutionChildren: React.FC = ({ + childGroups, + level }) => { - const taskExecutions = useDetailedTaskExecutions( - useTaskExecutions(nodeExecution.id) - ); - useTaskExecutionsRefresher(nodeExecution, taskExecutions); - - const columnStyles = useColumnStyles(); - // Memoizing columns so they won't be re-generated unless the styles change - const { columns, Skeleton } = React.useMemo(() => { - const columns = generateTaskExecutionColumns(columnStyles); - return { columns, Skeleton: generateRowSkeleton(columns) }; - }, [columnStyles]); - return ( - - {taskExecutions.value.length ? ( - taskExecutions.value.map(taskExecution => ( - - )) - ) : ( - - )} - - ); -}; - -interface WorkflowNodeExecution extends DetailedNodeExecution { - closure: NodeExecutionClosure & { - workflowNodeMetadata: WorkflowNodeMetadata; + const showNames = childGroups.length > 1; + const tableStyles = useExecutionTableStyles(); + const theme = useTheme(); + const childGroupLabelStyle = { + // The label is aligned with the parent above, so remove one level of spacing + marginLeft: `${calculateNodeExecutionRowLeftSpacing( + level - 1, + theme.spacing + )}px` }; -} -interface WorkflowNodeExecutionChildrenProps - extends NodeExecutionChildrenProps { - execution: WorkflowNodeExecution; -} - -const executionRefreshConfig: RefreshConfig = { - interval: executionRefreshIntervalMs, - valueIsFinal: executionIsTerminal -}; - -const WorkflowNodeExecutionChildren: React.FC = ({ - execution -}) => { - const { executionId } = execution.closure.workflowNodeMetadata; - const workflowExecution = useWorkflowExecution(executionId).fetchable; - useDataRefresher(executionId, workflowExecution, executionRefreshConfig); - - const columnStyles = useColumnStyles(); - // Memoizing columns so they won't be re-generated unless the styles change - const { columns, Skeleton } = React.useMemo(() => { - const columns = generateWorkflowExecutionColumns(columnStyles); - return { columns, Skeleton: generateRowSkeleton(columns) }; - }, [columnStyles]); return ( - - {workflowExecution.value ? ( - - ) : ( - - )} - + <> + {childGroups.map(({ name, nodeExecutions }, groupIndex) => { + const rows = nodeExecutions.map((nodeExecution, index) => ( + + )); + const key = `group-${name}`; + return showNames ? ( +
+
0 }, + tableStyles.borderBottom, + tableStyles.childGroupLabel + )} + style={childGroupLabelStyle} + > + + {name} + +
+
{rows}
+
+ ) : ( +
{rows}
+ ); + })} + ); }; - -/** Renders a nested list of row items for children of a NodeExecution */ -export const NodeExecutionChildren: React.FC = props => { - const { workflowNodeMetadata } = props.execution.closure; - const { execution: topExecution } = React.useContext(ExecutionContext); - - // Nested NodeExecutions will sometimes have `workflowNodeMetadata` that - // points to the parent WorkflowExecution. We only want to expand workflow - // nodes that point to a different workflow execution - if ( - workflowNodeMetadata && - !isEqual(workflowNodeMetadata.executionId, topExecution.id) - ) { - return ( - - ); - } - return ; -}; diff --git a/src/components/Executions/Tables/NodeExecutionRow.tsx b/src/components/Executions/Tables/NodeExecutionRow.tsx index 5dd64b1c9..ecafd66a8 100644 --- a/src/components/Executions/Tables/NodeExecutionRow.tsx +++ b/src/components/Executions/Tables/NodeExecutionRow.tsx @@ -1,44 +1,76 @@ import * as classnames from 'classnames'; -import { useExpandableMonospaceTextStyles } from 'components/common/ExpandableMonospaceText'; +import { useTheme } from 'components/Theme/useTheme'; import * as React from 'react'; +import { + ExecutionContext, + NodeExecutionsRequestConfigContext +} from '../contexts'; import { DetailedNodeExecution } from '../types'; +import { useChildNodeExecutions } from '../useChildNodeExecutions'; +import { useDetailedChildNodeExecutions } from '../useDetailedNodeExecutions'; import { NodeExecutionsTableContext } from './contexts'; import { ExpandableExecutionError } from './ExpandableExecutionError'; import { NodeExecutionChildren } from './NodeExecutionChildren'; import { RowExpander } from './RowExpander'; import { selectedClassName, useExecutionTableStyles } from './styles'; -import { NodeExecutionColumnDefinition } from './types'; +import { calculateNodeExecutionRowLeftSpacing } from './utils'; interface NodeExecutionRowProps { - columns: NodeExecutionColumnDefinition[]; + index: number; execution: DetailedNodeExecution; + level?: number; style?: React.CSSProperties; } -function isExpandableExecution(execution: DetailedNodeExecution) { - return true; -} - /** Renders a NodeExecution as a row inside a `NodeExecutionsTable` */ export const NodeExecutionRow: React.FC = ({ - columns, - execution, + execution: nodeExecution, + index, + level = 0, style }) => { - const state = React.useContext(NodeExecutionsTableContext); + const theme = useTheme(); + const { columns, state } = React.useContext(NodeExecutionsTableContext); + const requestConfig = React.useContext(NodeExecutionsRequestConfigContext); + const { execution: workflowExecution } = React.useContext(ExecutionContext); + const [expanded, setExpanded] = React.useState(false); const toggleExpanded = () => { setExpanded(!expanded); }; + + // For the first level, we want the borders to span the entire table, + // so we'll use padding to space the content. For nested rows, we want the + // border to start where the content does, so we'll use margin. + const spacingProp = level === 0 ? 'paddingLeft' : 'marginLeft'; + const rowContentStyle = { + [spacingProp]: `${calculateNodeExecutionRowLeftSpacing( + level, + theme.spacing + )}px` + }; + + // TODO: Handle error case for loading children. + // Maybe show an expander in that case and make the content the error? + const childNodeExecutions = useChildNodeExecutions({ + nodeExecution, + requestConfig, + workflowExecution + }); + + const detailedChildNodeExecutions = useDetailedChildNodeExecutions( + childNodeExecutions.value + ); + + const isExpandable = detailedChildNodeExecutions.length > 0; const tableStyles = useExecutionTableStyles(); - const monospaceTextStyles = useExpandableMonospaceTextStyles(); const selected = state.selectedExecution - ? state.selectedExecution === execution + ? state.selectedExecution === nodeExecution : false; - const { error } = execution.closure; + const { error } = nodeExecution.closure; - const expanderContent = isExpandableExecution(execution) ? ( + const expanderContent = isExpandable ? ( ) : null; @@ -48,17 +80,16 @@ export const NodeExecutionRow: React.FC = ({ const extraContent = expanded ? (
- {errorContent} - +
- ) : ( - errorContent - ); + ) : null; return (
= ({ })} style={style} > -
-
{expanderContent}
- {columns.map(({ className, key: columnKey, cellRenderer }) => ( +
0 && expanded), + [tableStyles.borderTop]: level > 0 && index > 0 + })} + style={rowContentStyle} + > +
- {cellRenderer({ - execution, - state - })} + {expanderContent}
- ))} + {columns.map( + ({ className, key: columnKey, cellRenderer }) => ( +
+ {cellRenderer({ + state, + execution: nodeExecution + })} +
+ ) + )} +
+ {errorContent}
{extraContent}
diff --git a/src/components/Executions/Tables/NodeExecutionsTable.tsx b/src/components/Executions/Tables/NodeExecutionsTable.tsx index 0bc651106..9d9ed3eae 100644 --- a/src/components/Executions/Tables/NodeExecutionsTable.tsx +++ b/src/components/Executions/Tables/NodeExecutionsTable.tsx @@ -2,9 +2,9 @@ import * as classnames from 'classnames'; import { DetailsPanel, ListProps } from 'components/common'; import { useCommonStyles } from 'components/common/styles'; import * as scrollbarSize from 'dom-helpers/util/scrollbarSize'; +import { NodeExecution } from 'models/Execution/types'; import * as React from 'react'; import { NodeExecutionDetails } from '../ExecutionDetails/NodeExecutionDetails'; -import { DetailedNodeExecution } from '../types'; import { NodeExecutionsTableContext } from './contexts'; import { ExecutionsTableHeader } from './ExecutionsTableHeader'; import { generateColumns } from './nodeExecutionColumns'; @@ -13,8 +13,7 @@ import { NoExecutionsContent } from './NoExecutionsContent'; import { useColumnStyles, useExecutionTableStyles } from './styles'; import { useNodeExecutionsTableState } from './useNodeExecutionsTableState'; -export interface NodeExecutionsTableProps - extends ListProps {} +export interface NodeExecutionsTableProps extends ListProps {} const scrollbarPadding = scrollbarSize(); @@ -36,13 +35,14 @@ export const NodeExecutionsTable: React.FC = props => const onCloseDetailsPanel = () => state.setSelectedExecution(null); - const rowProps = { columns, state, onHeightChange: () => {} }; + const rowProps = { state, onHeightChange: () => {} }; const content = state.executions.length > 0 ? ( - state.executions.map(execution => { + state.executions.map((execution, index) => { return ( @@ -63,7 +63,7 @@ export const NodeExecutionsTable: React.FC = props => columns={columns} scrollbarPadding={scrollbarPadding} /> - +
{content}
 = ({ - taskExecution -}) => { - const sort = { - key: executionSortFields.createdAt, - direction: SortDirection.ASCENDING - }; - const nodeExecutions = useDetailedNodeExecutions( - useTaskExecutionChildren(taskExecution.id, { - sort, - limit: limits.NONE - }) - ); - - const columnStyles = useColumnStyles(); - // Memoizing columns so they won't be re-generated unless the styles change - const { columns, Skeleton } = React.useMemo(() => { - const columns = generateColumns(columnStyles); - return { columns, Skeleton: generateRowSkeleton(columns) }; - }, [columnStyles]); - return ( - - {nodeExecutions.value.length ? ( - nodeExecutions.value.map(execution => ( - - )) - ) : ( - - )} - - ); -}; diff --git a/src/components/Executions/Tables/TaskExecutionRow.tsx b/src/components/Executions/Tables/TaskExecutionRow.tsx deleted file mode 100644 index 4cd519759..000000000 --- a/src/components/Executions/Tables/TaskExecutionRow.tsx +++ /dev/null @@ -1,87 +0,0 @@ -import * as classnames from 'classnames'; -import { useExpandableMonospaceTextStyles } from 'components/common/ExpandableMonospaceText'; -import { TaskExecution } from 'models'; -import * as React from 'react'; -import { DetailedNodeExecution, DetailedTaskExecution } from '../types'; -import { NodeExecutionsTableContext } from './contexts'; -import { ExpandableExecutionError } from './ExpandableExecutionError'; -import { RowExpander } from './RowExpander'; -import { useExecutionTableStyles } from './styles'; -import { TaskExecutionChildren } from './TaskExecutionChildren'; -import { TaskExecutionColumnDefinition } from './types'; - -interface TaskExecutionRowProps { - columns: TaskExecutionColumnDefinition[]; - execution: DetailedTaskExecution; - nodeExecution: DetailedNodeExecution; - style?: React.CSSProperties; -} - -function isExpandableExecution(execution: TaskExecution) { - return execution.isParent; -} - -/** Renders a TaskExecution as a row inside a `NodeExecutionsTable` */ -export const TaskExecutionRow: React.FC = ({ - columns, - execution, - nodeExecution, - style -}) => { - const state = React.useContext(NodeExecutionsTableContext); - const [expanded, setExpanded] = React.useState(false); - const toggleExpanded = () => { - setExpanded(!expanded); - }; - const tableStyles = useExecutionTableStyles(); - const monospaceTextStyles = useExpandableMonospaceTextStyles(); - const { error } = execution.closure; - - const expanderContent = isExpandableExecution(execution) ? ( - - ) : null; - - const errorContent = null; - // TODO: Show errors or remove this - // const errorContent = error ? ( - // - // ) : null; - - const extraContent = expanded ? ( -
- {errorContent} - -
- ) : ( - errorContent - ); - - return ( -
-
-
{expanderContent}
- {columns.map(({ className, key: columnKey, cellRenderer }) => ( -
- {cellRenderer({ - execution, - nodeExecution, - state - })} -
- ))} -
- {extraContent} -
- ); -}; diff --git a/src/components/Executions/Tables/WorkflowExecutionChildren.tsx b/src/components/Executions/Tables/WorkflowExecutionChildren.tsx deleted file mode 100644 index 753242188..000000000 --- a/src/components/Executions/Tables/WorkflowExecutionChildren.tsx +++ /dev/null @@ -1,48 +0,0 @@ -import { WaitForData } from 'components/common'; -import { waitForAllFetchables } from 'components/hooks'; -import { Execution } from 'models'; -import * as React from 'react'; -import { useWorkflowExecutionState } from '../useWorkflowExecutionState'; -import { generateRowSkeleton } from './generateRowSkeleton'; -import { generateColumns } from './nodeExecutionColumns'; -import { NodeExecutionRow } from './NodeExecutionRow'; -import { NoExecutionsContent } from './NoExecutionsContent'; -import { useColumnStyles } from './styles'; - -export interface WorkflowExecutionChildrenProps { - execution: Execution; -} - -/** Renders a nested list of row items for children of a WorkflowExecution */ -export const WorkflowExecutionChildren: React.FC = ({ - execution -}) => { - // Note: Not applying a filter to nested node executions - const { workflow, nodeExecutions } = useWorkflowExecutionState(execution); - - const columnStyles = useColumnStyles(); - // Memoizing columns so they won't be re-generated unless the styles change - const { columns, Skeleton } = React.useMemo(() => { - const columns = generateColumns(columnStyles); - return { columns, Skeleton: generateRowSkeleton(columns) }; - }, [columnStyles]); - return ( - - {nodeExecutions.value.length ? ( - nodeExecutions.value.map(execution => ( - - )) - ) : ( - - )} - - ); -}; diff --git a/src/components/Executions/Tables/WorkflowExecutionRow.tsx b/src/components/Executions/Tables/WorkflowExecutionRow.tsx deleted file mode 100644 index 6b0388fbb..000000000 --- a/src/components/Executions/Tables/WorkflowExecutionRow.tsx +++ /dev/null @@ -1,79 +0,0 @@ -import * as classnames from 'classnames'; -import { useExpandableMonospaceTextStyles } from 'components/common/ExpandableMonospaceText'; -import { Execution } from 'models/Execution'; -import * as React from 'react'; -import { NodeExecutionsTableContext } from './contexts'; -import { ExpandableExecutionError } from './ExpandableExecutionError'; -import { RowExpander } from './RowExpander'; -import { useExecutionTableStyles } from './styles'; -import { WorkflowExecutionColumnDefinition } from './types'; -import { WorkflowExecutionChildren } from './WorkflowExecutionChildren'; -// import { WorkflowExecutionChildren } from './WorkflowExecutionChildren'; - -interface WorkflowExecutionRowProps { - columns: WorkflowExecutionColumnDefinition[]; - execution: Execution; - style?: React.CSSProperties; -} - -function isExpandableExecution(execution: Execution) { - return true; -} - -/** Renders a WorkflowExecution as a row inside a `NodeExecutionsTable` */ -export const WorkflowExecutionRow: React.FC = ({ - columns, - execution, - style -}) => { - const state = React.useContext(NodeExecutionsTableContext); - const [expanded, setExpanded] = React.useState(false); - const toggleExpanded = () => { - setExpanded(!expanded); - }; - const tableStyles = useExecutionTableStyles(); - const monospaceTextStyles = useExpandableMonospaceTextStyles(); - const { error } = execution.closure; - - const expanderContent = isExpandableExecution(execution) ? ( - - ) : null; - - const errorContent = error ? ( - - ) : null; - - const extraContent = expanded ? ( -
- {errorContent} - -
- ) : ( - errorContent - ); - - return ( -
-
-
{expanderContent}
- {columns.map(({ className, key: columnKey, cellRenderer }) => ( -
- {cellRenderer({ - execution, - state - })} -
- ))} -
- {extraContent} -
- ); -}; diff --git a/src/components/Executions/Tables/WorkflowExecutionsTable.tsx b/src/components/Executions/Tables/WorkflowExecutionsTable.tsx index 5ebee3a6d..cc4b2b203 100644 --- a/src/components/Executions/Tables/WorkflowExecutionsTable.tsx +++ b/src/components/Executions/Tables/WorkflowExecutionsTable.tsx @@ -53,6 +53,9 @@ const useStyles = makeStyles((theme: Theme) => ({ marginLeft: theme.spacing(2), marginRight: theme.spacing(2), textAlign: 'left' + }, + row: { + paddingLeft: theme.spacing(2) } })); @@ -199,7 +202,14 @@ export const WorkflowExecutionsTable: React.FC = p const { error } = execution.closure; return ( -
+
{columns.map( ({ className, key: columnKey, cellRenderer }) => ( diff --git a/src/components/Executions/Tables/__stories__/NodeExecutionsTable.stories.tsx b/src/components/Executions/Tables/__stories__/NodeExecutionsTable.stories.tsx index 40d6b2f27..dd465ae06 100644 --- a/src/components/Executions/Tables/__stories__/NodeExecutionsTable.stories.tsx +++ b/src/components/Executions/Tables/__stories__/NodeExecutionsTable.stories.tsx @@ -1,20 +1,13 @@ import { makeStyles, Theme } from '@material-ui/core/styles'; import { action } from '@storybook/addon-actions'; import { storiesOf } from '@storybook/react'; -import axios from 'axios'; -// tslint:disable-next-line:import-name -import AxiosMockAdapter from 'axios-mock-adapter'; -import { mapNodeExecutionDetails } from 'components/Executions/utils'; -import { NavBar } from 'components/Navigation'; -import { Admin } from 'flyteidl'; -import { encodeProtoPayload } from 'models'; -import { - createMockWorkflow, - createMockWorkflowClosure -} from 'models/__mocks__/workflowData'; -import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData'; -import { createMockTaskExecutionsListResponse } from 'models/Execution/__mocks__/mockTaskExecutionsData'; -import { mockTasks } from 'models/Task/__mocks__/mockTaskData'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext } from 'components/data/apiContext'; +import { createMockExecutionEntities } from 'components/Executions/__mocks__/createMockExecutionEntities'; +import { ExecutionDataCacheContext } from 'components/Executions/contexts'; +import { createExecutionDataCache } from 'components/Executions/useExecutionDataCache'; +import { keyBy } from 'lodash'; +import { createMockTaskExecutionForNodeExecution } from 'models/Execution/__mocks__/mockTaskExecutionsData'; import * as React from 'react'; import { NodeExecutionsTable, @@ -32,25 +25,59 @@ const useStyles = makeStyles((theme: Theme) => ({ })); const { - executions: mockExecutions, - nodes: mockNodes -} = createMockNodeExecutions(10); + nodes, + nodeExecutions, + workflow, + workflowExecution +} = createMockExecutionEntities({ + workflowName: 'SampleWorkflow', + nodeExecutionCount: 10 +}); -const mockWorkflow = createMockWorkflow('SampleWorkflow'); -const mockWorkflowClosure = createMockWorkflowClosure(); -const compiledWorkflow = mockWorkflowClosure.compiledWorkflow!; -const { - primary: { template }, - tasks -} = compiledWorkflow; -template.nodes = template.nodes.concat(mockNodes); -compiledWorkflow.tasks = tasks.concat(mockTasks); -mockWorkflow.closure = mockWorkflowClosure; +const nodesById = keyBy(nodes, n => n.id); +const nodesWithChildren = { + [nodes[0].id]: true, + [nodes[1].id]: true +}; +const nodeRetryAttempts = { + [nodes[1].id]: 2 +}; + +const apiContext = mockAPIContextValue({ + getExecution: () => Promise.resolve(workflowExecution), + getNodeExecutionData: () => Promise.resolve({ inputs: {}, outputs: {} }), + listTaskExecutions: nodeExecutionId => { + const length = nodeRetryAttempts[nodeExecutionId.nodeId] || 1; + const entities = Array.from({ length }, (_, retryAttempt) => + createMockTaskExecutionForNodeExecution( + nodeExecutionId, + nodesById[nodeExecutionId.nodeId], + retryAttempt, + { isParent: !!nodesWithChildren[nodeExecutionId.nodeId] } + ) + ); + return Promise.resolve({ entities }); + }, + listTaskExecutionChildren: ({ retryAttempt }) => + Promise.resolve({ + entities: nodeExecutions.slice(0, 2).map(ne => ({ + ...ne, + id: { + ...ne.id, + nodeId: `${ne.id.nodeId}_${retryAttempt}` + } + })) + }), + getWorkflow: () => Promise.resolve(workflow) +}); +const dataCache = createExecutionDataCache(apiContext); +dataCache.insertWorkflow(workflow); +dataCache.insertWorkflowExecutionReference(workflowExecution.id, workflow.id); const fetchAction = action('fetch'); const props: NodeExecutionsTableProps = { - value: mapNodeExecutionDetails(mockExecutions, mockWorkflow), + value: nodeExecutions, lastError: null, loading: false, moreItemsAvailable: false, @@ -59,26 +86,15 @@ const props: NodeExecutionsTableProps = { const stories = storiesOf('Tables/NodeExecutionsTable', module); stories.addDecorator(story => { - React.useEffect(() => { - const executionsResponse = createMockTaskExecutionsListResponse(3); - const mock = new AxiosMockAdapter(axios); - mock.onGet(/.*\/task_executions\/.*/).reply(() => [ - 200, - encodeProtoPayload(executionsResponse, Admin.TaskExecutionList), - { 'Content-Type': 'application/octet-stream' } - ]); - return () => mock.restore(); - }); return ( - <> -
{story()}
- + + +
{story()}
+ + ); }); stories.add('Basic', () => ); -stories.add('With more items available', () => ( - -)); stories.add('With no items', () => ( )); diff --git a/src/components/Executions/Tables/contexts.ts b/src/components/Executions/Tables/contexts.ts index ad2b55e43..b2be1892c 100644 --- a/src/components/Executions/Tables/contexts.ts +++ b/src/components/Executions/Tables/contexts.ts @@ -1,6 +1,14 @@ import * as React from 'react'; -import { NodeExecutionsTableState } from './types'; +import { + NodeExecutionColumnDefinition, + NodeExecutionsTableState +} from './types'; + +export interface NodeExecutionsTableContextData { + columns: NodeExecutionColumnDefinition[]; + state: NodeExecutionsTableState; +} export const NodeExecutionsTableContext = React.createContext< - NodeExecutionsTableState ->({} as NodeExecutionsTableState); + NodeExecutionsTableContextData +>({} as NodeExecutionsTableContextData); diff --git a/src/components/Executions/Tables/nodeExecutionColumns.tsx b/src/components/Executions/Tables/nodeExecutionColumns.tsx index 519405542..2082bbc7b 100644 --- a/src/components/Executions/Tables/nodeExecutionColumns.tsx +++ b/src/components/Executions/Tables/nodeExecutionColumns.tsx @@ -49,7 +49,14 @@ export function generateColumns( ): NodeExecutionColumnDefinition[] { return [ { - cellRenderer: props => , + cellRenderer: props => ( + <> + + + {props.execution.displayId} + + + ), className: styles.columnName, key: 'name', label: 'node' diff --git a/src/components/Executions/Tables/styles.ts b/src/components/Executions/Tables/styles.ts index 97f5f04f1..763392aba 100644 --- a/src/components/Executions/Tables/styles.ts +++ b/src/components/Executions/Tables/styles.ts @@ -3,6 +3,7 @@ import { headerGridHeight } from 'components'; import { headerFontFamily, listhoverColor, + nestedListColor, smallFontSize, tableHeaderColor, tablePlaceholderColor @@ -16,23 +17,32 @@ export const selectedClassName = 'selected'; // the columns styles in some cases. So the column styles should be defined // last. export const useExecutionTableStyles = makeStyles((theme: Theme) => ({ + borderBottom: { + borderBottom: `1px solid ${theme.palette.divider}` + }, + borderTop: { + borderTop: `1px solid ${theme.palette.divider}` + }, childrenContainer: { - borderTop: `1px solid ${theme.palette.divider}`, - minHeight: theme.spacing(7), - paddingLeft: theme.spacing(3) + backgroundColor: nestedListColor, + minHeight: theme.spacing(7) + }, + childGroupLabel: { + borderWidth: '2px', + padding: `${theme.spacing(2)}px 0` }, errorContainer: { - padding: `0 ${theme.spacing(7)}px ${theme.spacing(2)}px`, - '$childrenContainer > &': { + padding: `0 ${theme.spacing(8)}px ${theme.spacing(2)}px`, + '$childrenContainer &': { paddingTop: theme.spacing(2), - paddingLeft: theme.spacing(4) + paddingLeft: theme.spacing(2) } }, expander: { alignItems: 'center', display: 'flex', justifyContent: 'center', - marginLeft: theme.spacing(3), + marginLeft: theme.spacing(-4), marginRight: theme.spacing(1), width: theme.spacing(3) }, @@ -75,20 +85,14 @@ export const useExecutionTableStyles = makeStyles((theme: Theme) => ({ textAlign: 'center' }, row: { - borderBottom: `1px solid ${theme.palette.divider}`, display: 'flex', flexDirection: 'column', justifyContent: 'center', [`&.${selectedClassName}`]: { backgroundColor: listhoverColor - }, - '$childrenContainer &': { - borderBottom: 'none', - '&:not(:first-of-type)': { - borderTop: `1px solid ${theme.palette.divider}` - } } }, + rowContent: {}, rowColumns: { alignItems: 'center', display: 'flex', @@ -102,10 +106,7 @@ export const useExecutionTableStyles = makeStyles((theme: Theme) => ({ paddingBottom: theme.spacing(2), paddingTop: theme.spacing(2), textOverflow: 'ellipsis', - whiteSpace: 'nowrap', - '&:first-of-type': { - marginLeft: theme.spacing(2) - } + whiteSpace: 'nowrap' }, scrollContainer: { flex: '1 1 0', @@ -118,6 +119,7 @@ export const useExecutionTableStyles = makeStyles((theme: Theme) => ({ } })); +export const nameColumnLeftMarginGridWidth = 6; export const useColumnStyles = makeStyles((theme: Theme) => ({ columnName: { flexGrow: 1, @@ -126,7 +128,7 @@ export const useColumnStyles = makeStyles((theme: Theme) => ({ flexBasis: 0, overflow: 'hidden', '&:first-of-type': { - marginLeft: theme.spacing(7) + marginLeft: theme.spacing(nameColumnLeftMarginGridWidth) } }, columnType: { diff --git a/src/components/Executions/Tables/taskExecutionColumns.tsx b/src/components/Executions/Tables/taskExecutionColumns.tsx deleted file mode 100644 index 4847c0293..000000000 --- a/src/components/Executions/Tables/taskExecutionColumns.tsx +++ /dev/null @@ -1,180 +0,0 @@ -import { Typography } from '@material-ui/core'; -import { - formatDateLocalTimezone, - formatDateUTC, - millisecondsToHMS -} from 'common/formatters'; -import { timestampToDate } from 'common/utils'; -import { NewTargetLink } from 'components/common'; -import { useCommonStyles } from 'components/common/styles'; -import { useTheme } from 'components/Theme/useTheme'; -import { TaskExecutionPhase } from 'models/Execution/enums'; -import * as React from 'react'; -import { ExecutionStatusBadge, getTaskExecutionTimingMS } from '..'; -import { noLogsFoundString } from '../constants'; -import { getUniqueTaskExecutionName } from '../TaskExecutionsList/utils'; -import { nodeExecutionsTableColumnWidths } from './constants'; -import { SelectNodeExecutionLink } from './SelectNodeExecutionLink'; -import { useColumnStyles, useExecutionTableStyles } from './styles'; -import { - TaskExecutionCellRendererData, - TaskExecutionColumnDefinition -} from './types'; -import { splitLogLinksAtWidth } from './utils'; - -const TaskExecutionLogLinks: React.FC = ({ - execution, - nodeExecution, - state -}) => { - const tableStyles = useExecutionTableStyles(); - const commonStyles = useCommonStyles(); - const { logs = [] } = execution.closure; - const { measureTextWidth } = useTheme(); - const measuredLogs = React.useMemo( - () => - logs.map(log => ({ - ...log, - width: measureTextWidth('body1', log.name) - })), - [logs, measureTextWidth] - ); - - if (measuredLogs.length === 0) { - return ( - {noLogsFoundString} - ); - } - - // Leaving room at the end to render the "xxx More" string - const [taken, left] = splitLogLinksAtWidth( - measuredLogs, - nodeExecutionsTableColumnWidths.logs - 56 - ); - - // If we don't have enough room to render any individual links, just - // show the selection link to open the details panel - if (!taken.length) { - return ( - - ); - } - - return ( -
- {taken.map(({ name, uri }, index) => ( - - {name} - - ))} - {left.length === 0 ? null : ( - - )} -
- ); -}; - -export function generateColumns( - styles: ReturnType -): TaskExecutionColumnDefinition[] { - return [ - { - cellRenderer: ({ execution }) => - getUniqueTaskExecutionName(execution), - className: styles.columnName, - key: 'name', - label: 'node' - }, - { - cellRenderer: ({ - execution: { - closure: { phase = TaskExecutionPhase.UNDEFINED } - } - }) => , - className: styles.columnStatus, - key: 'phase', - label: 'status' - }, - { - cellRenderer: () => 'Task Execution', - className: styles.columnType, - key: 'type', - label: 'type' - }, - { - cellRenderer: ({ execution: { closure } }) => { - const { startedAt } = closure; - if (!startedAt) { - return ''; - } - const startedAtDate = timestampToDate(startedAt); - return ( - <> - - {formatDateUTC(startedAtDate)} - - - {formatDateLocalTimezone(startedAtDate)} - - - ); - }, - className: styles.columnStartedAt, - key: 'startedAt', - label: 'start time' - }, - { - cellRenderer: ({ execution }) => { - const timing = getTaskExecutionTimingMS(execution); - if (timing === null) { - return ''; - } - return ( - <> - - {millisecondsToHMS(timing.duration)} - - - {millisecondsToHMS(timing.queued)} - - - ); - }, - className: styles.columnDuration, - key: 'duration', - label: () => ( - <> - - duration - - - Queued Time - - - ) - }, - { - cellRenderer: props => , - className: styles.columnLogs, - key: 'logs', - label: 'logs' - } - ]; -} diff --git a/src/components/Executions/Tables/test/NodeExecutionChildren.test.tsx b/src/components/Executions/Tables/test/NodeExecutionChildren.test.tsx deleted file mode 100644 index b68650762..000000000 --- a/src/components/Executions/Tables/test/NodeExecutionChildren.test.tsx +++ /dev/null @@ -1,65 +0,0 @@ -import { render, waitFor } from '@testing-library/react'; -import { noExecutionsFoundString } from 'common/constants'; -import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; -import { APIContext } from 'components/data/apiContext'; -import { - DetailedNodeExecution, - NodeExecutionDisplayType -} from 'components/Executions/types'; -import { - listTaskExecutions, - NodeExecution, - SortDirection, - taskSortFields -} from 'models'; -import { mockNodeExecutionResponse } from 'models/Execution/__mocks__/mockNodeExecutionsData'; -import * as React from 'react'; -import { NodeExecutionChildren } from '../NodeExecutionChildren'; - -describe('NodeExecutionChildren', () => { - let nodeExecution: DetailedNodeExecution; - let mockListTaskExecutions: jest.Mock>; - - const renderChildren = () => - render( - - - - ); - beforeEach(() => { - nodeExecution = { - ...(mockNodeExecutionResponse as NodeExecution), - displayId: 'testNode', - displayType: NodeExecutionDisplayType.PythonTask, - cacheKey: 'abcdefg' - }; - mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] }); - }); - - it('Renders message when no task executions exist', async () => { - const { queryByText } = renderChildren(); - await waitFor(() => {}); - expect(mockListTaskExecutions).toHaveBeenCalled(); - expect(queryByText(noExecutionsFoundString)).toBeInTheDocument(); - }); - - it('Requests items in correct order', async () => { - renderChildren(); - await waitFor(() => {}); - expect(mockListTaskExecutions).toHaveBeenCalledWith( - expect.anything(), - expect.objectContaining({ - sort: { - key: taskSortFields.createdAt, - direction: SortDirection.ASCENDING - } - }) - ); - }); -}); diff --git a/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx new file mode 100644 index 000000000..b8e3d072f --- /dev/null +++ b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx @@ -0,0 +1,152 @@ +import { render, waitFor } from '@testing-library/react'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext, APIContextValue } from 'components/data/apiContext'; +import { createMockExecutionEntities } from 'components/Executions/__mocks__/createMockExecutionEntities'; +import { + ExecutionContext, + ExecutionContextData, + ExecutionDataCacheContext, + NodeExecutionsRequestConfigContext +} from 'components/Executions/contexts'; +import { ExecutionDataCache } from 'components/Executions/types'; +import { createExecutionDataCache } from 'components/Executions/useExecutionDataCache'; +import { + FilterOperationName, + getTask, + NodeExecution, + RequestConfig, + WorkflowExecutionIdentifier +} from 'models'; +import { createMockExecution } from 'models/__mocks__/executionsData'; +import { createMockTaskExecutionsListResponse } from 'models/Execution/__mocks__/mockTaskExecutionsData'; +import { + getExecution, + listTaskExecutionChildren, + listTaskExecutions +} from 'models/Execution/api'; +import { mockTasks } from 'models/Task/__mocks__/mockTaskData'; +import * as React from 'react'; +import { Identifier } from 'typescript'; +import { + NodeExecutionsTable, + NodeExecutionsTableProps +} from '../NodeExecutionsTable'; + +describe('NodeExecutionsTable', () => { + let props: NodeExecutionsTableProps; + let apiContext: APIContextValue; + let executionContext: ExecutionContextData; + let dataCache: ExecutionDataCache; + let requestConfig: RequestConfig; + let mockNodeExecutions: NodeExecution[]; + let mockGetExecution: jest.Mock>; + let mockGetTask: jest.Mock>; + let mockListTaskExecutions: jest.Mock>; + let mockListTaskExecutionChildren: jest.Mock>; + + beforeEach(() => { + const { + nodeExecutions, + workflow, + workflowExecution + } = createMockExecutionEntities({ + workflowName: 'SampleWorkflow', + nodeExecutionCount: 2 + }); + + mockNodeExecutions = nodeExecutions; + + mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] }); + mockListTaskExecutionChildren = jest + .fn() + .mockResolvedValue({ entities: [] }); + mockGetExecution = jest + .fn() + .mockImplementation(async (id: WorkflowExecutionIdentifier) => { + return { ...createMockExecution(id.name), id }; + }); + mockGetTask = jest.fn().mockImplementation(async (id: Identifier) => { + return { template: { ...mockTasks[0].template, id } }; + }); + + apiContext = mockAPIContextValue({ + getExecution: mockGetExecution, + getTask: mockGetTask, + listTaskExecutions: mockListTaskExecutions, + listTaskExecutionChildren: mockListTaskExecutionChildren + }); + + dataCache = createExecutionDataCache(apiContext); + dataCache.insertWorkflow(workflow); + dataCache.insertWorkflowExecutionReference( + workflowExecution.id, + workflow.id + ); + + requestConfig = {}; + executionContext = { + execution: workflowExecution, + terminateExecution: jest.fn().mockRejectedValue('Not Implemented') + }; + + props = { + value: nodeExecutions, + lastError: null, + loading: false, + moreItemsAvailable: false, + fetch: jest.fn() + }; + }); + + const renderTable = () => + render( + + + + + + + + + + ); + + it('renders task name for task nodes', async () => { + const { queryAllByText } = renderTable(); + await waitFor(() => {}); + + const node = dataCache.getNodeForNodeExecution( + mockNodeExecutions[0].id + ); + const taskId = node?.node.taskNode?.referenceId; + expect(taskId).toBeDefined(); + const task = dataCache.getTaskTemplate(taskId!); + expect(task).toBeDefined(); + expect(queryAllByText(task!.id.name)[0]).toBeInTheDocument(); + }); + + it('requests child node executions using configuration from context', async () => { + const { taskExecutions } = createMockTaskExecutionsListResponse(1); + taskExecutions[0].isParent = true; + mockListTaskExecutions.mockResolvedValue({ entities: taskExecutions }); + requestConfig.filter = [ + { key: 'test', operation: FilterOperationName.EQ, value: 'test' } + ]; + + renderTable(); + await waitFor(() => + expect(mockListTaskExecutionChildren).toHaveBeenCalled() + ); + + expect(mockListTaskExecutionChildren).toHaveBeenCalledWith( + taskExecutions[0].id, + expect.objectContaining(requestConfig) + ); + }); +}); diff --git a/src/components/Executions/Tables/useNodeExecutionsTableState.ts b/src/components/Executions/Tables/useNodeExecutionsTableState.ts index 15c278e21..579d44abb 100644 --- a/src/components/Executions/Tables/useNodeExecutionsTableState.ts +++ b/src/components/Executions/Tables/useNodeExecutionsTableState.ts @@ -1,12 +1,18 @@ -import { useState } from 'react'; +import { useContext, useMemo, useState } from 'react'; +import { ExecutionDataCacheContext } from '../contexts'; import { DetailedNodeExecution } from '../types'; +import { mapNodeExecutionDetails } from '../utils'; import { NodeExecutionsTableProps } from './NodeExecutionsTable'; import { NodeExecutionsTableState } from './types'; -export function useNodeExecutionsTableState( - props: NodeExecutionsTableProps -): NodeExecutionsTableState { - const { value: executions } = props; +export function useNodeExecutionsTableState({ + value +}: NodeExecutionsTableProps): NodeExecutionsTableState { + const dataCache = useContext(ExecutionDataCacheContext); + const executions = useMemo( + () => mapNodeExecutionDetails(value, dataCache), + [value] + ); const [ selectedExecution, diff --git a/src/components/Executions/Tables/utils.ts b/src/components/Executions/Tables/utils.ts index 28edc9bce..5e097b23c 100644 --- a/src/components/Executions/Tables/utils.ts +++ b/src/components/Executions/Tables/utils.ts @@ -1,5 +1,7 @@ +import { Spacing } from '@material-ui/core/styles/createSpacing'; import { measureText } from 'components/common/utils'; import { TaskLog } from 'models'; +import { nameColumnLeftMarginGridWidth } from './styles'; interface MeasuredTaskLog extends TaskLog { width: number; @@ -35,3 +37,10 @@ export function splitLogLinksAtWidth( ); return [taken, left]; } + +export function calculateNodeExecutionRowLeftSpacing( + level: number, + spacing: Spacing +) { + return spacing(nameColumnLeftMarginGridWidth + 3 * level); +} diff --git a/src/components/Executions/Tables/workflowExecutionColumns.tsx b/src/components/Executions/Tables/workflowExecutionColumns.tsx deleted file mode 100644 index d8e793d5c..000000000 --- a/src/components/Executions/Tables/workflowExecutionColumns.tsx +++ /dev/null @@ -1,101 +0,0 @@ -import { Typography } from '@material-ui/core'; -import { - formatDateLocalTimezone, - formatDateUTC, - millisecondsToHMS -} from 'common/formatters'; -import { timestampToDate } from 'common/utils'; -import { WorkflowExecutionPhase } from 'models/Execution/enums'; -import * as React from 'react'; -import { ExecutionStatusBadge, getWorkflowExecutionTimingMS } from '..'; -import { useColumnStyles } from './styles'; -import { WorkflowExecutionColumnDefinition } from './types'; - -export function generateColumns( - styles: ReturnType -): WorkflowExecutionColumnDefinition[] { - return [ - { - cellRenderer: ({ execution }) => execution.id.name, - className: styles.columnName, - key: 'name', - label: 'node' - }, - { - cellRenderer: ({ - execution: { - closure: { phase = WorkflowExecutionPhase.UNDEFINED } - } - }) => , - className: styles.columnStatus, - key: 'phase', - label: 'status' - }, - { - cellRenderer: () => 'Workflow Execution', - className: styles.columnType, - key: 'type', - label: 'type' - }, - { - cellRenderer: ({ execution: { closure } }) => { - const { startedAt } = closure; - if (!startedAt) { - return ''; - } - const startedAtDate = timestampToDate(startedAt); - return ( - <> - - {formatDateUTC(startedAtDate)} - - - {formatDateLocalTimezone(startedAtDate)} - - - ); - }, - className: styles.columnStartedAt, - key: 'startedAt', - label: 'start time' - }, - { - cellRenderer: ({ execution }) => { - const timing = getWorkflowExecutionTimingMS(execution); - if (timing === null) { - return ''; - } - return ( - <> - - {millisecondsToHMS(timing.duration)} - - - ); - }, - className: styles.columnDuration, - key: 'duration', - label: () => ( - <> - - duration - - - Queued Time - - - ) - }, - { - // TODO: Implement this content - cellRenderer: () => null, - className: styles.columnLogs, - key: 'logs', - label: 'logs' - } - ]; -} diff --git a/src/components/Executions/TaskExecutionDetails/TaskExecutionDetails.tsx b/src/components/Executions/TaskExecutionDetails/TaskExecutionDetails.tsx index a67e24249..ba6da5250 100644 --- a/src/components/Executions/TaskExecutionDetails/TaskExecutionDetails.tsx +++ b/src/components/Executions/TaskExecutionDetails/TaskExecutionDetails.tsx @@ -7,6 +7,8 @@ import { import { TaskExecution, TaskExecutionIdentifier } from 'models'; import * as React from 'react'; import { executionRefreshIntervalMs } from '../constants'; +import { ExecutionDataCacheContext } from '../contexts'; +import { useExecutionDataCache } from '../useExecutionDataCache'; import { taskExecutionIsTerminal } from '../utils'; import { TaskExecutionDetailsAppBarContent } from './TaskExecutionDetailsAppBarContent'; import { TaskExecutionNodes } from './TaskExecutionNodes'; @@ -58,6 +60,7 @@ function routeParamsToTaskExecutionId( export const TaskExecutionDetailsContainer: React.FC = props => { const taskExecutionId = routeParamsToTaskExecutionId(props); + const dataCache = useExecutionDataCache(); const taskExecution = useTaskExecution(taskExecutionId); useDataRefresher(taskExecutionId, taskExecution, refreshConfig); @@ -67,7 +70,9 @@ export const TaskExecutionDetailsContainer: React.FC - + + + ); }; diff --git a/src/components/Executions/TaskExecutionDetails/TaskExecutionNodes.tsx b/src/components/Executions/TaskExecutionDetails/TaskExecutionNodes.tsx index e48d2380b..df7529e5f 100644 --- a/src/components/Executions/TaskExecutionDetails/TaskExecutionNodes.tsx +++ b/src/components/Executions/TaskExecutionDetails/TaskExecutionNodes.tsx @@ -1,21 +1,24 @@ import { Tab, Tabs } from '@material-ui/core'; import { makeStyles, Theme } from '@material-ui/core/styles'; import { WaitForData } from 'components/common'; -import { useDataRefresher, useTaskExecutionChildren } from 'components/hooks'; +import { useDataRefresher, useFetchableData } from 'components/hooks'; import { useTabState } from 'components/hooks/useTabState'; import { every } from 'lodash'; import { executionSortFields, limits, + NodeExecution, + RequestConfig, SortDirection, - TaskExecution + TaskExecution, + TaskExecutionIdentifier } from 'models'; import * as React from 'react'; import { executionRefreshIntervalMs, nodeExecutionIsTerminal } from '..'; +import { ExecutionDataCacheContext } from '../contexts'; import { ExecutionFilters } from '../ExecutionFilters'; import { useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState'; import { NodeExecutionsTable } from '../Tables/NodeExecutionsTable'; -import { useDetailedNodeExecutions } from '../useDetailedNodeExecutions'; import { taskExecutionIsTerminal } from '../utils'; const useStyles = makeStyles((theme: Theme) => ({ @@ -41,6 +44,28 @@ const tabIds = { nodes: 'nodes' }; +interface UseCachedTaskExecutionChildrenArgs { + config: RequestConfig; + id: TaskExecutionIdentifier; +} +function useCachedTaskExecutionChildren( + args: UseCachedTaskExecutionChildrenArgs +) { + const dataCache = React.useContext(ExecutionDataCacheContext); + return useFetchableData< + NodeExecution[], + UseCachedTaskExecutionChildrenArgs + >( + { + debugName: 'CachedTaskExecutionChildren', + defaultValue: [], + doFetch: ({ id, config }) => + dataCache.getTaskExecutionChildren(id, config) + }, + args + ); +} + /** Contains the content for viewing child NodeExecutions for a TaskExecution */ export const TaskExecutionNodes: React.FC = ({ taskExecution @@ -52,13 +77,14 @@ export const TaskExecutionNodes: React.FC = ({ key: executionSortFields.createdAt, direction: SortDirection.ASCENDING }; - const nodeExecutions = useDetailedNodeExecutions( - useTaskExecutionChildren(taskExecution.id, { + const nodeExecutions = useCachedTaskExecutionChildren({ + id: taskExecution.id, + config: { sort, limit: limits.NONE, filter: filterState.appliedFilters - }) - ); + } + }); // We will continue to refresh the node executions list as long // as either the parent execution or any child is non-terminal diff --git a/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx b/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx index 0229cb2f5..52356de1e 100644 --- a/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx +++ b/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx @@ -11,7 +11,7 @@ import { ExecutionStatusBadge } from '../ExecutionStatusBadge'; import { TaskExecutionDetails } from './TaskExecutionDetails'; import { TaskExecutionError } from './TaskExecutionError'; import { TaskExecutionLogs } from './TaskExecutionLogs'; -import { getUniqueTaskExecutionName } from './utils'; +import { formatRetryAttempt } from './utils'; const useStyles = makeStyles((theme: Theme) => ({ detailsLink: { @@ -43,7 +43,7 @@ export const TaskExecutionsListItem: React.FC = ({ const styles = useStyles(); const { closure, isParent } = taskExecution; const { error } = closure; - const headerText = getUniqueTaskExecutionName(taskExecution); + const headerText = formatRetryAttempt(taskExecution.id.retryAttempt); const taskHasStarted = closure.phase >= TaskExecutionPhase.QUEUED; return (
diff --git a/src/components/Executions/TaskExecutionsList/test/utils.spec.ts b/src/components/Executions/TaskExecutionsList/test/utils.spec.ts index a4711a2f4..616581479 100644 --- a/src/components/Executions/TaskExecutionsList/test/utils.spec.ts +++ b/src/components/Executions/TaskExecutionsList/test/utils.spec.ts @@ -1,5 +1,5 @@ import { obj } from 'test/utils'; -import { getUniqueTaskExecutionName } from '../utils'; +import { formatRetryAttempt, getUniqueTaskExecutionName } from '../utils'; describe('getUniqueTaskExecutionName', () => { const cases: [{ name: string; retryAttempt: number }, string][] = [ @@ -20,3 +20,16 @@ describe('getUniqueTaskExecutionName', () => { ).toEqual(expected)) ); }); + +describe('formatRetryAttempt', () => { + const cases: [number, string][] = [ + [0, 'Attempt 01'], + [1, 'Attempt 02'], + [2, 'Attempt 03'] + ]; + + cases.forEach(([input, expected]) => + it(`should return ${expected} for ${input}`, () => + expect(formatRetryAttempt(input)).toEqual(expected)) + ); +}); diff --git a/src/components/Executions/TaskExecutionsList/utils.ts b/src/components/Executions/TaskExecutionsList/utils.ts index 84e54c685..407a29026 100644 --- a/src/components/Executions/TaskExecutionsList/utils.ts +++ b/src/components/Executions/TaskExecutionsList/utils.ts @@ -1,3 +1,4 @@ +import { leftPaddedNumber } from 'common/formatters'; import { TaskExecution } from 'models'; /** Generates a unique name for a task execution, suitable for display in a @@ -11,3 +12,8 @@ export function getUniqueTaskExecutionName({ id }: TaskExecution) { const suffix = retryAttempt > 0 ? ` (${retryAttempt + 1})` : ''; return `${name}${suffix}`; } + +export function formatRetryAttempt(attempt: number): string { + // Retry attempts are zero-based, so incrementing before formatting + return `Attempt ${leftPaddedNumber(attempt + 1, 2)}`; +} diff --git a/src/components/Executions/TerminateExecution/useTerminateExecutionState.ts b/src/components/Executions/TerminateExecution/useTerminateExecutionState.ts index b428434d0..9129147af 100644 --- a/src/components/Executions/TerminateExecution/useTerminateExecutionState.ts +++ b/src/components/Executions/TerminateExecution/useTerminateExecutionState.ts @@ -1,5 +1,5 @@ import { useContext, useState } from 'react'; -import { ExecutionContext } from '../ExecutionDetails/contexts'; +import { ExecutionContext } from '../contexts'; /** Holds state for `TerminateExecutionForm` */ export function useTerminateExecutionState(onClose: () => void) { diff --git a/src/components/Executions/__mocks__/createMockExecutionEntities.ts b/src/components/Executions/__mocks__/createMockExecutionEntities.ts new file mode 100644 index 000000000..fd9e7021b --- /dev/null +++ b/src/components/Executions/__mocks__/createMockExecutionEntities.ts @@ -0,0 +1,45 @@ +import { cloneDeep } from 'lodash'; +import { + createMockWorkflow, + createMockWorkflowClosure +} from 'models/__mocks__/workflowData'; +import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData'; +import { mockExecution as mockWorkflowExecution } from 'models/Execution/__mocks__/mockWorkflowExecutionsData'; +import { mockTasks } from 'models/Task/__mocks__/mockTaskData'; + +interface CreateExecutionEntitiesArgs { + workflowName: string; + nodeExecutionCount: number; +} + +/** Creates the basic entities necessary to render the majority of the + * ExecutionDetails page. These can be inserted into an ExecutionDataCache + * for mocking. + */ +export function createMockExecutionEntities({ + workflowName, + nodeExecutionCount +}: CreateExecutionEntitiesArgs) { + const { executions: nodeExecutions, nodes } = createMockNodeExecutions( + nodeExecutionCount + ); + + const workflow = createMockWorkflow(workflowName); + const workflowClosure = createMockWorkflowClosure(); + const compiledWorkflow = workflowClosure.compiledWorkflow!; + const { + primary: { template }, + tasks + } = compiledWorkflow; + template.nodes = template.nodes.concat(nodes); + compiledWorkflow.tasks = tasks.concat(cloneDeep(mockTasks)); + workflow.closure = workflowClosure; + + return { + nodes, + nodeExecutions, + tasks, + workflow, + workflowExecution: mockWorkflowExecution + }; +} diff --git a/src/components/Executions/ExecutionDetails/contexts.ts b/src/components/Executions/contexts.ts similarity index 52% rename from src/components/Executions/ExecutionDetails/contexts.ts rename to src/components/Executions/contexts.ts index a1c69244d..3a3afdebc 100644 --- a/src/components/Executions/ExecutionDetails/contexts.ts +++ b/src/components/Executions/contexts.ts @@ -1,14 +1,24 @@ import * as React from 'react'; -import { Execution, NodeExecution } from 'models'; +import { Execution, NodeExecution, RequestConfig } from 'models'; +import { ExecutionDataCache } from './types'; export interface ExecutionContextData { execution: Execution; terminateExecution(cause: string): Promise; } + export const ExecutionContext = React.createContext( {} as ExecutionContextData ); export const NodeExecutionsContext = React.createContext< Dictionary >({}); + +export const NodeExecutionsRequestConfigContext = React.createContext< + RequestConfig +>({}); + +export const ExecutionDataCacheContext = React.createContext< + ExecutionDataCache +>({} as ExecutionDataCache); diff --git a/src/components/Executions/types.ts b/src/components/Executions/types.ts index d0af41d25..a6e13f7d6 100644 --- a/src/components/Executions/types.ts +++ b/src/components/Executions/types.ts @@ -1,4 +1,20 @@ -import { NodeExecution, TaskExecution } from 'models/Execution/types'; +import { + CompiledNode, + GloballyUniqueNode, + Identifier, + NodeId, + RequestConfig, + Workflow, + WorkflowId +} from 'models'; +import { + Execution, + NodeExecution, + NodeExecutionIdentifier, + TaskExecution, + TaskExecutionIdentifier, + WorkflowExecutionIdentifier +} from 'models/Execution/types'; import { TaskTemplate } from 'models/Task/types'; export interface ExecutionPhaseConstants { @@ -22,6 +38,15 @@ export enum NodeExecutionDisplayType { WaitableTask = 'Waitable Task' } +export interface UniqueNodeId { + workflowId: WorkflowId; + nodeId: string; +} +export interface NodeInformation { + id: UniqueNodeId; + node: CompiledNode; +} + /** An interface combining a NodeExecution with data pulled from the * corresponding Workflow Node structure. */ @@ -35,3 +60,46 @@ export interface DetailedNodeExecution extends NodeExecution { export interface DetailedTaskExecution extends TaskExecution { cacheKey: string; } + +export interface NodeExecutionGroup { + name: string; + nodeExecutions: NodeExecution[]; +} + +export interface DetailedNodeExecutionGroup extends NodeExecutionGroup { + nodeExecutions: DetailedNodeExecution[]; +} + +export interface ExecutionDataCache { + getNode(id: NodeId): GloballyUniqueNode | undefined; + getNodeForNodeExecution( + nodeExecutionId: NodeExecutionIdentifier + ): GloballyUniqueNode | null | undefined; + getNodeExecutions( + workflowExecutionId: WorkflowExecutionIdentifier, + config: RequestConfig + ): Promise; + getTaskExecutions( + nodeExecutionId: NodeExecutionIdentifier + ): Promise; + getTaskExecutionChildren: ( + taskExecutionId: TaskExecutionIdentifier, + config: RequestConfig + ) => Promise; + getTaskTemplate: (taskId: Identifier) => TaskTemplate | undefined; + getWorkflow: (workflowId: Identifier) => Promise; + getWorkflowExecution: ( + executionId: WorkflowExecutionIdentifier + ) => Promise; + getWorkflowIdForWorkflowExecution: ( + executionId: WorkflowExecutionIdentifier + ) => Promise; + insertExecution(execution: Execution): void; + insertNodes(nodes: GloballyUniqueNode[]): void; + insertTaskTemplates(templates: TaskTemplate[]): void; + insertWorkflow(workflow: Workflow): void; + insertWorkflowExecutionReference( + executionId: WorkflowExecutionIdentifier, + workflowId: WorkflowId + ): void; +} diff --git a/src/components/Executions/useChildNodeExecutions.ts b/src/components/Executions/useChildNodeExecutions.ts new file mode 100644 index 000000000..70cd4ad95 --- /dev/null +++ b/src/components/Executions/useChildNodeExecutions.ts @@ -0,0 +1,153 @@ +import { FetchableData } from 'components/hooks'; +import { useFetchableData } from 'components/hooks/useFetchableData'; +import { isEqual } from 'lodash'; +import { + Execution, + NodeExecution, + RequestConfig, + TaskExecutionIdentifier, + WorkflowExecutionIdentifier +} from 'models'; +import { useContext } from 'react'; +import { ExecutionContext, ExecutionDataCacheContext } from './contexts'; +import { formatRetryAttempt } from './TaskExecutionsList/utils'; +import { ExecutionDataCache, NodeExecutionGroup } from './types'; + +interface FetchGroupForTaskExecutionArgs { + config: RequestConfig; + dataCache: ExecutionDataCache; + taskExecutionId: TaskExecutionIdentifier; +} +async function fetchGroupForTaskExecution({ + dataCache, + config, + taskExecutionId +}: FetchGroupForTaskExecutionArgs): Promise { + return { + // NodeExecutions created by a TaskExecution are grouped + // by the retry attempt of the task. + name: formatRetryAttempt(taskExecutionId.retryAttempt), + nodeExecutions: await dataCache.getTaskExecutionChildren( + taskExecutionId, + config + ) + }; +} + +interface FetchGroupForWorkflowExecutionArgs { + config: RequestConfig; + dataCache: ExecutionDataCache; + workflowExecutionId: WorkflowExecutionIdentifier; +} +async function fetchGroupForWorkflowExecution({ + config, + dataCache, + workflowExecutionId +}: FetchGroupForWorkflowExecutionArgs): Promise { + return { + // NodeExecutions created by a workflow execution are grouped + // by the execution id, since workflow executions are not retryable. + name: workflowExecutionId.name, + nodeExecutions: await dataCache.getNodeExecutions( + workflowExecutionId, + config + ) + }; +} + +interface FetchNodeExecutionGroupArgs { + config: RequestConfig; + dataCache: ExecutionDataCache; + nodeExecution: NodeExecution; +} + +async function fetchGroupsForTaskExecutionNode({ + config, + dataCache, + nodeExecution: { id: nodeExecutionId } +}: FetchNodeExecutionGroupArgs): Promise { + const taskExecutions = await dataCache.getTaskExecutions(nodeExecutionId); + + // For TaskExecutions marked as parents, fetch its children and create a group. + // Otherwise, return null and we will filter it out later. + const groups = await Promise.all( + taskExecutions.map(execution => + execution.isParent + ? fetchGroupForTaskExecution({ + dataCache, + config, + taskExecutionId: execution.id + }) + : Promise.resolve(null) + ) + ); + + // Remove any empty groups + return groups.filter( + group => group !== null && group.nodeExecutions.length > 0 + ) as NodeExecutionGroup[]; +} + +async function fetchGroupsForWorkflowExecutionNode({ + config, + dataCache, + nodeExecution +}: FetchNodeExecutionGroupArgs): Promise { + if (!nodeExecution.closure.workflowNodeMetadata) { + throw new Error('Unexpected empty `workflowNodeMetadata`'); + } + const { + executionId: workflowExecutionId + } = nodeExecution.closure.workflowNodeMetadata; + // We can only have one WorkflowExecution (no retries), so there is only + // one group to return. But calling code expects it as an array. + const group = await fetchGroupForWorkflowExecution({ + dataCache, + config, + workflowExecutionId + }); + return group.nodeExecutions.length > 0 ? [group] : []; +} + +export interface UseChildNodeExecutionsArgs { + requestConfig: RequestConfig; + nodeExecution: NodeExecution; + workflowExecution: Execution; +} + +/** Fetches and groups `NodeExecution`s which are direct children of the given + * `NodeExecution`. + */ +export function useChildNodeExecutions({ + nodeExecution, + requestConfig +}: UseChildNodeExecutionsArgs): FetchableData { + const { execution: topExecution } = useContext(ExecutionContext); + const dataCache = useContext(ExecutionDataCacheContext); + const { workflowNodeMetadata } = nodeExecution.closure; + return useFetchableData( + { + debugName: 'ChildNodeExecutions', + defaultValue: [], + doFetch: async data => { + const fetchArgs = { + dataCache, + config: requestConfig, + nodeExecution: data + }; + + // Nested NodeExecutions will sometimes have `workflowNodeMetadata` that + // points to the parent WorkflowExecution. We're only interested in + // showing children if this node is a sub-workflow. + if ( + workflowNodeMetadata && + !isEqual(workflowNodeMetadata.executionId, topExecution.id) + ) { + return fetchGroupsForWorkflowExecutionNode(fetchArgs); + } + return fetchGroupsForTaskExecutionNode(fetchArgs); + } + }, + nodeExecution + ); +} diff --git a/src/components/Executions/useDetailedNodeExecutions.ts b/src/components/Executions/useDetailedNodeExecutions.ts index c7125ceb2..d1bdb7f7b 100644 --- a/src/components/Executions/useDetailedNodeExecutions.ts +++ b/src/components/Executions/useDetailedNodeExecutions.ts @@ -1,31 +1,38 @@ -import { useNodeExecutions, useWorkflow } from 'components/hooks'; -import { Workflow } from 'models'; -import { useMemo } from 'react'; +import { NodeExecution } from 'models'; +import { useContext, useMemo } from 'react'; +import { ExecutionDataCacheContext } from './contexts'; +import { NodeExecutionGroup } from './types'; import { mapNodeExecutionDetails } from './utils'; -/** Decorates a fetchable list of NodeExecutions generated by `useNodeExecutions`, - * mapping the list items to `DetailedNodeExecution`s. The node details are - * pulled from the closure of a fetchable `Workflow`. - * Note: This hook can generate output without a valid workflow value (i.e. before - * the workflow has finished fetching). It is the responsibility of calling code - * to use `waitForData` or `waitForAllFetchables` to prevent display of the - * output if this would be undesirable. +/** Decorates a list of NodeExecutions, mapping the list items to + * `DetailedNodeExecution`s. The node details are pulled from the the nearest + * `ExecutionContext.dataCache`. */ -export function useDetailedNodeExecutions( - nodeExecutionsFetchable: ReturnType, - workflowFetchable?: ReturnType -) { - const { value: nodeExecutions } = nodeExecutionsFetchable; - let workflow: Workflow | undefined = undefined; - if (workflowFetchable && workflowFetchable.hasLoaded) { - workflow = workflowFetchable.value; - } +export function useDetailedNodeExecutions(nodeExecutions: NodeExecution[]) { + const dataCache = useContext(ExecutionDataCacheContext); + + return useMemo(() => mapNodeExecutionDetails(nodeExecutions, dataCache), [ + nodeExecutions, + dataCache + ]); +} - return { - ...nodeExecutionsFetchable, - value: useMemo( - () => mapNodeExecutionDetails(nodeExecutions, workflow), - [nodeExecutions, workflow] - ) - }; +/** Decorates a list of `NodeExecutionGroup`s, transforming their lists of + * `NodeExecution`s into `DetailedNodeExecution`s. + */ +export function useDetailedChildNodeExecutions( + nodeExecutionGroups: NodeExecutionGroup[] +) { + const dataCache = useContext(ExecutionDataCacheContext); + return useMemo( + () => + nodeExecutionGroups.map(group => ({ + ...group, + nodeExecutions: mapNodeExecutionDetails( + group.nodeExecutions, + dataCache + ) + })), + [nodeExecutionGroups, dataCache] + ); } diff --git a/src/components/Executions/useExecutionDataCache.ts b/src/components/Executions/useExecutionDataCache.ts new file mode 100644 index 000000000..325cf4af4 --- /dev/null +++ b/src/components/Executions/useExecutionDataCache.ts @@ -0,0 +1,242 @@ +import { log } from 'common/log'; +import { getCacheKey } from 'components/Cache'; +import { APIContextValue, useAPIContext } from 'components/data/apiContext'; +import { + fetchNodeExecutions, + fetchTaskExecutionChildren +} from 'components/hooks'; +import { + extractAndIdentifyNodes, + extractTaskTemplates +} from 'components/hooks/utils'; +import { NotFoundError } from 'errors'; +import { + Execution, + GloballyUniqueNode, + Identifier, + NodeExecutionIdentifier, + NodeId, + RequestConfig, + TaskExecutionIdentifier, + TaskTemplate, + Workflow, + WorkflowExecutionIdentifier, + WorkflowId +} from 'models'; +import { useState } from 'react'; +import { ExecutionDataCache } from './types'; +import { fetchTaskExecutions } from './useTaskExecutions'; + +function cacheItems( + map: Map, + values: T[] +) { + values.forEach(v => map.set(getCacheKey(v.id), v)); +} + +/** Creates a new ExecutionDataCache which will use the provided API context + * to fetch entities. + */ +export function createExecutionDataCache( + apiContext: APIContextValue +): ExecutionDataCache { + const workflowsById: Map = new Map(); + const nodesById: Map = new Map(); + const taskTemplatesById: Map = new Map(); + const workflowExecutionIdToWorkflowId: Map = new Map(); + + const insertNodes = (nodes: GloballyUniqueNode[]) => { + cacheItems(nodesById, nodes); + }; + + const insertTaskTemplates = (templates: TaskTemplate[]) => { + cacheItems(taskTemplatesById, templates); + }; + + const insertWorkflow = (workflow: Workflow) => { + workflowsById.set(getCacheKey(workflow.id), workflow); + insertNodes(extractAndIdentifyNodes(workflow)); + insertTaskTemplates(extractTaskTemplates(workflow)); + }; + + const insertExecution = (execution: Execution) => { + workflowExecutionIdToWorkflowId.set( + getCacheKey(execution.id), + execution.closure.workflowId + ); + }; + + const insertWorkflowExecutionReference = ( + executionId: WorkflowExecutionIdentifier, + workflowId: WorkflowId + ) => { + workflowExecutionIdToWorkflowId.set( + getCacheKey(executionId), + workflowId + ); + }; + + const getWorkflow = async (id: WorkflowId) => { + const key = getCacheKey(id); + if (workflowsById.has(key)) { + return workflowsById.get(key)!; + } + const workflow = await apiContext.getWorkflow(id); + insertWorkflow(workflow); + return workflow; + }; + + const getNode = (id: NodeId) => { + const node = nodesById.get(getCacheKey(id)); + if (node === undefined) { + log.error('Unexpected Node missing from cache:', id); + } + return node; + }; + + const getNodeForNodeExecution = ({ + executionId, + nodeId + }: NodeExecutionIdentifier) => { + const workflowExecutionKey = getCacheKey(executionId); + if (!workflowExecutionIdToWorkflowId.has(workflowExecutionKey)) { + log.error( + 'Unexpected missing parent workflow execution: ', + executionId + ); + return null; + } + const workflowId = workflowExecutionIdToWorkflowId.get( + workflowExecutionKey + )!; + return getNode({ nodeId, workflowId }); + }; + + const getNodeExecutions = async ( + id: WorkflowExecutionIdentifier, + config: RequestConfig + ) => { + const execution = await getWorkflowExecution(id); + // Fetching workflow to ensure node definitions exist + const [_, nodeExecutions] = await Promise.all([ + getWorkflow(execution.closure.workflowId), + fetchNodeExecutions({ config, id }, apiContext) + ]); + return nodeExecutions; + }; + + const getTaskTemplate = (id: Identifier) => { + const template = taskTemplatesById.get(getCacheKey(id)); + if (template === undefined) { + log.error('Unexpected TaskTemplate missing from cache:', id); + } + return template; + }; + + const getOrFetchTaskTemplate = async (id: Identifier) => { + const key = getCacheKey(id); + if (taskTemplatesById.has(key)) { + return taskTemplatesById.get(key); + } + try { + const { template } = ( + await apiContext.getTask(id) + ).closure.compiledTask; + taskTemplatesById.set(key, template); + return template; + } catch (e) { + if (e instanceof NotFoundError) { + log.warn('No task template found for task: ', id); + return; + } + throw e; + } + }; + + const getWorkflowExecution = async (id: WorkflowExecutionIdentifier) => { + const execution = await apiContext.getExecution(id); + insertWorkflowExecutionReference( + execution.id, + execution.closure.workflowId + ); + return execution; + }; + + const getWorkflowIdForWorkflowExecution = async ( + id: WorkflowExecutionIdentifier + ) => { + const key = getCacheKey(id); + if (workflowExecutionIdToWorkflowId.has(key)) { + return workflowExecutionIdToWorkflowId.get(key)!; + } + return (await getWorkflowExecution(id)).closure.workflowId; + }; + + const getTaskExecutions = async (id: NodeExecutionIdentifier) => + fetchTaskExecutions(id, apiContext); + + const getTaskExecutionChildren = async ( + id: TaskExecutionIdentifier, + config: RequestConfig + ) => { + const childrenPromise = fetchTaskExecutionChildren( + { config, taskExecutionId: id }, + apiContext + ); + const workflowIdPromise = getWorkflowIdForWorkflowExecution( + id.nodeExecutionId.executionId + ); + + const cacheTaskTemplatePromise = await getOrFetchTaskTemplate( + id.taskId + ); + + const [children, workflowId, _] = await Promise.all([ + childrenPromise, + workflowIdPromise, + cacheTaskTemplatePromise + ]); + + // We need to synthesize a record for each child node, + // as they won't exist in any Workflow closure. + const nodes = children.map(node => ({ + id: { + workflowId, + nodeId: node.id.nodeId + }, + node: { + id: node.id.nodeId, + taskNode: { + referenceId: id.taskId + } + } + })); + insertNodes(nodes); + + return children; + }; + + return { + getNode, + getNodeForNodeExecution, + getNodeExecutions, + getTaskExecutions, + getTaskExecutionChildren, + getTaskTemplate, + getWorkflow, + getWorkflowExecution, + getWorkflowIdForWorkflowExecution, + insertExecution, + insertNodes, + insertTaskTemplates, + insertWorkflow, + insertWorkflowExecutionReference + }; +} + +/** A hook for creating a new ExecutionDataCache wired to the nearest `APIContext` */ +export function useExecutionDataCache() { + const apiContext = useAPIContext(); + const [dataCache] = useState(() => createExecutionDataCache(apiContext)); + return dataCache; +} diff --git a/src/components/Executions/useTaskExecutions.ts b/src/components/Executions/useTaskExecutions.ts index 4d616cb6b..32f459b24 100644 --- a/src/components/Executions/useTaskExecutions.ts +++ b/src/components/Executions/useTaskExecutions.ts @@ -1,4 +1,4 @@ -import { useAPIContext } from 'components/data/apiContext'; +import { APIContextValue, useAPIContext } from 'components/data/apiContext'; import { every } from 'lodash'; import { ExecutionData, @@ -16,27 +16,38 @@ import { useFetchableData } from '../hooks/useFetchableData'; import { executionRefreshIntervalMs } from './constants'; import { nodeExecutionIsTerminal, taskExecutionIsTerminal } from './utils'; +/** Fetches a list of `TaskExecution`s which are children of the given `NodeExecution`. + * This function is meant to be consumed by hooks which are composing data. + * If you're calling it from a component, consider using `useTaskExecutions` instead. + */ +export const fetchTaskExecutions = async ( + id: NodeExecutionIdentifier, + apiContext: APIContextValue +) => { + const { listTaskExecutions } = apiContext; + const { entities } = await listTaskExecutions(id, { + limit: limits.NONE, + sort: { + key: taskSortFields.createdAt, + direction: SortDirection.ASCENDING + } + }); + return entities; +}; + /** A hook for fetching the list of TaskExecutions associated with a * NodeExecution */ export function useTaskExecutions( id: NodeExecutionIdentifier ): FetchableData { - const { listTaskExecutions } = useAPIContext(); + const apiContext = useAPIContext(); return useFetchableData( { debugName: 'TaskExecutions', defaultValue: [], - doFetch: async (id: NodeExecutionIdentifier) => { - const { entities } = await listTaskExecutions(id, { - limit: limits.NONE, - sort: { - key: taskSortFields.createdAt, - direction: SortDirection.ASCENDING - } - }); - return entities; - } + doFetch: async (id: NodeExecutionIdentifier) => + fetchTaskExecutions(id, apiContext) }, id ); @@ -57,6 +68,9 @@ export function useTaskExecutionData( ); } +/** Wraps the result of `useTaskExecutions` and will refresh the data as long + * as the given `NodeExecution` is still in a non-final state. + */ export function useTaskExecutionsRefresher( nodeExecution: NodeExecution, taskExecutionsFetchable: ReturnType diff --git a/src/components/hooks/useWorkflowExecution.ts b/src/components/Executions/useWorkflowExecution.ts similarity index 88% rename from src/components/hooks/useWorkflowExecution.ts rename to src/components/Executions/useWorkflowExecution.ts index 47fdc5020..b5f7e4d42 100644 --- a/src/components/hooks/useWorkflowExecution.ts +++ b/src/components/Executions/useWorkflowExecution.ts @@ -9,18 +9,20 @@ import { import { useAPIContext } from 'components/data/apiContext'; import { maxBlobDownloadSizeBytes } from 'components/Literals/constants'; -import { FetchableData, FetchableExecution } from './types'; -import { useFetchableData } from './useFetchableData'; +import { FetchableData, FetchableExecution } from '../hooks/types'; +import { useFetchableData } from '../hooks/useFetchableData'; +import { ExecutionDataCache } from './types'; /** A hook for fetching a WorkflowExecution */ export function useWorkflowExecution( - id: WorkflowExecutionIdentifier + id: WorkflowExecutionIdentifier, + dataCache: ExecutionDataCache ): FetchableExecution { const fetchable = useFetchableData( { debugName: 'Execution', defaultValue: {} as Execution, - doFetch: id => getExecution(id) + doFetch: id => dataCache.getWorkflowExecution(id) }, id ); diff --git a/src/components/Executions/useWorkflowExecutionState.ts b/src/components/Executions/useWorkflowExecutionState.ts index a1331cb60..d42b5c38b 100644 --- a/src/components/Executions/useWorkflowExecutionState.ts +++ b/src/components/Executions/useWorkflowExecutionState.ts @@ -1,7 +1,7 @@ import { useDataRefresher, - useNodeExecutions, - useWorkflow + useFetchableData, + useNodeExecutions } from 'components/hooks'; import { every } from 'lodash'; import { @@ -9,14 +9,33 @@ import { executionSortFields, FilterOperation, limits, - SortDirection + SortDirection, + Workflow, + WorkflowId } from 'models'; +import { useContext } from 'react'; import { executionIsTerminal, executionRefreshIntervalMs, nodeExecutionIsTerminal } from '.'; -import { useDetailedNodeExecutions } from './useDetailedNodeExecutions'; +import { ExecutionDataCacheContext } from './contexts'; + +/** Using a custom fetchable to make sure the related workflow is fetched + * using an ExecutionDataCache, ensuring that the extended details for NodeExecutions + * can be found. + */ +function useCachedWorkflow(id: WorkflowId) { + const dataCache = useContext(ExecutionDataCacheContext); + return useFetchableData( + { + debugName: 'Workflow', + defaultValue: {} as Workflow, + doFetch: id => dataCache.getWorkflow(id) + }, + id + ); +} /** Fetches both the workflow and nodeExecutions for a given WorkflowExecution. * Will also map node details to the node executions. @@ -29,25 +48,26 @@ export function useWorkflowExecutionState( key: executionSortFields.createdAt, direction: SortDirection.ASCENDING }; - const rawNodeExecutions = useNodeExecutions(execution.id, { + const nodeExecutionsRequestConfig = { filter, sort, limit: limits.NONE - }); - const workflow = useWorkflow(execution.closure.workflowId); - const nodeExecutions = useDetailedNodeExecutions( - rawNodeExecutions, - workflow + }; + const nodeExecutions = useNodeExecutions( + execution.id, + nodeExecutionsRequestConfig ); + const workflow = useCachedWorkflow(execution.closure.workflowId); + // We will continue to refresh the node executions list as long // as either the parent execution or any child is non-terminal useDataRefresher(execution.id, nodeExecutions, { interval: executionRefreshIntervalMs, - valueIsFinal: nodeExecutions => - every(nodeExecutions, nodeExecutionIsTerminal) && + valueIsFinal: executions => + every(executions, nodeExecutionIsTerminal) && executionIsTerminal(execution) }); - return { workflow, nodeExecutions }; + return { workflow, nodeExecutions, nodeExecutionsRequestConfig }; } diff --git a/src/components/Executions/utils.ts b/src/components/Executions/utils.ts index 2f0346448..bc0c4d436 100644 --- a/src/components/Executions/utils.ts +++ b/src/components/Executions/utils.ts @@ -1,3 +1,13 @@ +import { log } from 'common/log'; +import { durationToMilliseconds, timestampToDate } from 'common/utils'; +import { getCacheKey } from 'components/Cache'; +import { + endNodeId, + Identifier, + startNodeId, + TaskTemplate, + TaskType +} from 'models'; import { BaseExecutionClosure, Execution, @@ -7,27 +17,11 @@ import { terminalNodeExecutionStates, terminalTaskExecutionStates } from 'models/Execution'; - import { NodeExecutionPhase, TaskExecutionPhase, WorkflowExecutionPhase } from 'models/Execution/enums'; - -import { log } from 'common/log'; -import { durationToMilliseconds, timestampToDate } from 'common/utils'; -import { getCacheKey } from 'components/Cache'; -import { extractTaskTemplates } from 'components/hooks/utils'; -import { keyBy } from 'lodash'; -import { - CompiledNode, - endNodeId, - Identifier, - startNodeId, - TaskTemplate, - TaskType, - Workflow -} from 'models'; import { nodeExecutionPhaseConstants, taskExecutionPhaseConstants, @@ -36,6 +30,7 @@ import { } from './constants'; import { DetailedNodeExecution, + ExecutionDataCache, ExecutionPhaseConstants, NodeExecutionDisplayType } from './types'; @@ -114,96 +109,81 @@ export const taskExecutionIsTerminal = (taskExecution: TaskExecution) => taskExecution.closure && terminalTaskExecutionStates.includes(taskExecution.closure.phase); -/** Assigns display information to NodeExecutions. Each NodeExecution has an - * associated `nodeId`. If a `Workflow` is provided, node/task information will - * be pulled from the workflow closure to determine the node type. - */ -export function mapNodeExecutionDetails( - executions: NodeExecution[], - workflow?: Workflow +/** Populates a NodeExecution with extended information read from an `ExecutionDataCache` */ +export function populateNodeExecutionDetails( + nodeExecution: NodeExecution, + dataCache: ExecutionDataCache ) { - let nodesById: Dictionary = {}; - let taskTemplates: Dictionary = {}; + const { nodeId } = nodeExecution.id; + const cacheKey = getCacheKey(nodeExecution.id); + const nodeInfo = dataCache.getNodeForNodeExecution(nodeExecution.id); - if (workflow) { - if (!workflow.closure) { - throw new Error('Workflow has no closure'); + let displayId = nodeId; + let displayType = NodeExecutionDisplayType.Unknown; + let taskTemplate: TaskTemplate | undefined = undefined; + + if (nodeInfo == null) { + return { ...nodeExecution, cacheKey, displayId, displayType }; + } + const { node } = nodeInfo; + + if (node.branchNode) { + displayId = nodeId; + displayType = NodeExecutionDisplayType.BranchNode; + } else if (node.taskNode) { + displayType = NodeExecutionDisplayType.UnknownTask; + taskTemplate = dataCache.getTaskTemplate(node.taskNode.referenceId); + + if (!taskTemplate) { + displayType = NodeExecutionDisplayType.UnknownTask; + } else { + displayId = taskTemplate.id.name; + displayType = + taskTypeToNodeExecutionDisplayType[ + taskTemplate.type as TaskType + ]; + if (!displayType) { + displayType = NodeExecutionDisplayType.UnknownTask; + } } - if (!workflow.closure.compiledWorkflow) { - throw new Error('Workflow closure missing a compiled workflow'); + } else if (node.workflowNode) { + displayType = NodeExecutionDisplayType.Workflow; + const { launchplanRef, subWorkflowRef } = node.workflowNode; + const identifier = (launchplanRef + ? launchplanRef + : subWorkflowRef) as Identifier; + if (!identifier) { + log.warn(`Unexpected workflow node with no ref: ${nodeId}`); + } else { + displayId = identifier.name; } - - taskTemplates = keyBy(extractTaskTemplates(workflow), t => - getCacheKey(t.id) - ); - nodesById = keyBy( - workflow.closure.compiledWorkflow.primary.template.nodes, - 'id' - ); } + return { + ...nodeExecution, + cacheKey, + displayId, + displayType, + taskTemplate + }; +} + +/** Assigns display information to NodeExecutions. Each NodeExecution has an + * associated `nodeId`. Extended details are populated using the provided `ExecutionDataCache` + */ +export function mapNodeExecutionDetails( + executions: NodeExecution[], + dataCache: ExecutionDataCache +) { return executions .filter(execution => { // Exclude the start/end nodes from the renderered list const { nodeId } = execution.id; return !(nodeId === startNodeId || nodeId === endNodeId); }) - .map(execution => { - const { nodeId } = execution.id; - const node = nodesById[nodeId]; - const cacheKey = getCacheKey(execution.id); - let displayId = nodeId; - let displayType = NodeExecutionDisplayType.Unknown; - let taskTemplate: TaskTemplate | undefined = undefined; - - if (!node) { - return { ...execution, cacheKey, displayId, displayType }; - } - - if (node.branchNode) { - displayId = nodeId; - displayType = NodeExecutionDisplayType.BranchNode; - } else if (node.taskNode) { - displayType = NodeExecutionDisplayType.UnknownTask; - taskTemplate = - taskTemplates[getCacheKey(node.taskNode.referenceId)]; - - if (!taskTemplate) { - log.warn( - `Unexpected missing workflow task for node ${nodeId}` - ); - displayType = NodeExecutionDisplayType.UnknownTask; - } else { - displayId = taskTemplate.id.name; - displayType = - taskTypeToNodeExecutionDisplayType[ - taskTemplate.type as TaskType - ]; - if (!displayType) { - displayType = NodeExecutionDisplayType.UnknownTask; - } - } - } else if (node.workflowNode) { - displayType = NodeExecutionDisplayType.Workflow; - const { launchplanRef, subWorkflowRef } = node.workflowNode; - const identifier = (launchplanRef - ? launchplanRef - : subWorkflowRef) as Identifier; - if (!identifier) { - log.warn(`Unexpected workflow node with no ref: ${nodeId}`); - } else { - displayId = identifier.name; - } - } - - return { - ...execution, - cacheKey, - displayId, - displayType, - taskTemplate - }; - }); + .map(execution => + populateNodeExecutionDetails(execution, dataCache) + ); } interface GetExecutionDurationMSArgs { diff --git a/src/components/Launch/LaunchWorkflowForm/useExecutionLaunchConfiguration.ts b/src/components/Launch/LaunchWorkflowForm/useExecutionLaunchConfiguration.ts index fe0f1d54a..ba5b40fa9 100644 --- a/src/components/Launch/LaunchWorkflowForm/useExecutionLaunchConfiguration.ts +++ b/src/components/Launch/LaunchWorkflowForm/useExecutionLaunchConfiguration.ts @@ -1,5 +1,6 @@ import { log } from 'common/log'; -import { FetchableData, useWorkflowExecutionInputs } from 'components/hooks'; +import { useWorkflowExecutionInputs } from 'components/Executions/useWorkflowExecution'; +import { FetchableData } from 'components/hooks'; import { Execution, Variable } from 'models'; import { useEffect, useState } from 'react'; import { InitialLaunchParameters, LiteralValueMap } from './types'; diff --git a/src/components/common/scopedContext.ts b/src/components/common/scopedContext.ts new file mode 100644 index 000000000..e69de29bb diff --git a/src/components/data/__mocks__/apiContext.ts b/src/components/data/__mocks__/apiContext.ts index 39bb0aa11..995dea144 100644 --- a/src/components/data/__mocks__/apiContext.ts +++ b/src/components/data/__mocks__/apiContext.ts @@ -12,7 +12,9 @@ export function mockAPIContextValue( >( (out, key) => Object.assign(out, { - [key]: () => Promise.reject(` ${key} not implemented`) + [key]: () => { + throw new Error(` ${key} not implemented`); + } }), {} as APIContextValue ); diff --git a/src/components/hooks/index.ts b/src/components/hooks/index.ts index 7cacf6045..2b9cbb891 100644 --- a/src/components/hooks/index.ts +++ b/src/components/hooks/index.ts @@ -10,7 +10,6 @@ export * from './useRemoteLiteralMap'; export * from './useTask'; export * from './useTaskExecution'; export * from './useWorkflow'; -export * from './useWorkflowExecution'; export * from './useWorkflowExecutions'; export * from './useWorkflows'; export * from './useWorkflowSchedules'; diff --git a/src/components/hooks/test/utils.test.ts b/src/components/hooks/test/utils.test.ts new file mode 100644 index 000000000..5b8027719 --- /dev/null +++ b/src/components/hooks/test/utils.test.ts @@ -0,0 +1,79 @@ +import { + createMockWorkflow, + createMockWorkflowClosure +} from 'models/__mocks__/workflowData'; +import { CompiledWorkflow, Workflow } from 'models/Workflow/types'; +import { extractAndIdentifyNodes, extractTaskTemplates } from '../utils'; + +describe('hooks/utils', () => { + let workflow: Workflow; + let subWorkflow: CompiledWorkflow; + + beforeEach(() => { + workflow = { + ...createMockWorkflow('SampleWorkflow'), + closure: createMockWorkflowClosure() + }; + const { template } = workflow.closure!.compiledWorkflow!.primary; + subWorkflow = { + // We don't process connections in these functions, so it can be empty + connections: { downstream: {}, upstream: {} }, + template: { + id: { ...template.id, name: `${template.id.name}_inner` }, + nodes: template.nodes.map(node => ({ + ...node, + id: `${node.id}_inner` + })) + } + }; + workflow.closure!.compiledWorkflow!.subWorkflows = [subWorkflow]; + }); + + describe('extractAndIdentifyNodes', () => { + it('returns empty for missing closure', () => { + delete workflow.closure; + expect(extractAndIdentifyNodes(workflow)).toEqual([]); + }); + + it('returns empty for missing compiledWorkflow', () => { + delete workflow.closure?.compiledWorkflow; + expect(extractAndIdentifyNodes(workflow)).toEqual([]); + }); + + it('includes nodes from subWorkflows', () => { + const nodes = extractAndIdentifyNodes(workflow); + subWorkflow.template.nodes.forEach(node => + expect(nodes).toContainEqual( + expect.objectContaining({ + id: { nodeId: node.id, workflowId: expect.anything() } + }) + ) + ); + }); + + it('assigns parent workflow id to subworkflow nodes', () => { + const nodes = extractAndIdentifyNodes(workflow); + subWorkflow.template.nodes.forEach(node => + expect(nodes).toContainEqual( + expect.objectContaining({ + id: { + nodeId: expect.anything(), + workflowId: workflow.id + } + }) + ) + ); + }); + }); + + describe('extractTaskTemplates', () => { + it('returns empty for missing closure', () => { + delete workflow.closure; + expect(extractTaskTemplates(workflow)).toEqual([]); + }); + it('returns empty for missing compiledWorkflow', () => { + delete workflow.closure?.compiledWorkflow; + expect(extractTaskTemplates(workflow)).toEqual([]); + }); + }); +}); diff --git a/src/components/hooks/useNodeExecution.ts b/src/components/hooks/useNodeExecution.ts index 821ed4657..e89457486 100644 --- a/src/components/hooks/useNodeExecution.ts +++ b/src/components/hooks/useNodeExecution.ts @@ -1,11 +1,5 @@ -import { - ExecutionData, - getNodeExecution, - getNodeExecutionData, - NodeExecution, - NodeExecutionIdentifier -} from 'models'; - +import { useAPIContext } from 'components/data/apiContext'; +import { ExecutionData, NodeExecution, NodeExecutionIdentifier } from 'models'; import { FetchableData } from './types'; import { useFetchableData } from './useFetchableData'; @@ -13,6 +7,7 @@ import { useFetchableData } from './useFetchableData'; export function useNodeExecution( id: NodeExecutionIdentifier ): FetchableData { + const { getNodeExecution } = useAPIContext(); return useFetchableData( { debugName: 'NodeExecution', @@ -27,6 +22,7 @@ export function useNodeExecution( export function useNodeExecutionData( id: NodeExecutionIdentifier ): FetchableData { + const { getNodeExecutionData } = useAPIContext(); return useFetchableData( { debugName: 'NodeExecutionData', diff --git a/src/components/hooks/useNodeExecutions.ts b/src/components/hooks/useNodeExecutions.ts index 06260ce5e..0a8433d02 100644 --- a/src/components/hooks/useNodeExecutions.ts +++ b/src/components/hooks/useNodeExecutions.ts @@ -1,7 +1,6 @@ +import { APIContextValue, useAPIContext } from 'components/data/apiContext'; import { limits, - listNodeExecutions, - listTaskExecutionChildren, NodeExecution, RequestConfig, TaskExecutionIdentifier, @@ -19,10 +18,15 @@ interface TaskExecutionChildrenFetchData { config: RequestConfig; } -const doFetchNodeExecutions = async ({ - id, - config -}: NodeExecutionsFetchData) => { +/** Fetches a list of `NodeExecution`s which are children of a WorkflowExecution. + * This function is meant to be consumed by hooks which are composing data. + * If you're calling it from a component, consider using `useNodeExecutions` instead. + */ +export const fetchNodeExecutions = async ( + { id, config }: NodeExecutionsFetchData, + apiContext: APIContextValue +) => { + const { listNodeExecutions } = apiContext; const { entities } = await listNodeExecutions(id, { ...config, limit: limits.NONE @@ -37,20 +41,26 @@ export function useNodeExecutions( id: WorkflowExecutionIdentifier, config: RequestConfig ) { + const apiContext = useAPIContext(); return useFetchableData( { debugName: 'NodeExecutions', defaultValue: [], - doFetch: doFetchNodeExecutions + doFetch: data => fetchNodeExecutions(data, apiContext) }, { id, config } ); } -const doFetchTaskExecutionChildren = async ({ - taskExecutionId, - config -}: TaskExecutionChildrenFetchData) => { +/** Fetches a list of `NodeExecution`s which are children of the given `TaskExecution`. + * This function is meant to be consumed by hooks which are composing data. + * If you're calling it from a component, consider using `useTaskExecutionChildren` instead. + */ +export const fetchTaskExecutionChildren = async ( + { taskExecutionId, config }: TaskExecutionChildrenFetchData, + apiContext: APIContextValue +) => { + const { listTaskExecutionChildren } = apiContext; const { entities } = await listTaskExecutionChildren(taskExecutionId, { ...config, limit: limits.NONE @@ -63,11 +73,12 @@ export function useTaskExecutionChildren( taskExecutionId: TaskExecutionIdentifier, config: RequestConfig ) { + const apiContext = useAPIContext(); return useFetchableData( { debugName: 'TaskExecutionChildren', defaultValue: [], - doFetch: doFetchTaskExecutionChildren + doFetch: data => fetchTaskExecutionChildren(data, apiContext) }, { taskExecutionId, config } ); diff --git a/src/components/hooks/useTask.ts b/src/components/hooks/useTask.ts index c23e4f948..368501bb2 100644 --- a/src/components/hooks/useTask.ts +++ b/src/components/hooks/useTask.ts @@ -1,5 +1,4 @@ import { useAPIContext } from 'components/data/apiContext'; -import { NotFoundError } from 'errors'; import { Identifier, IdentifierScope, @@ -11,22 +10,20 @@ import { FetchableData } from './types'; import { useFetchableData } from './useFetchableData'; import { usePagination } from './usePagination'; -/** A hook for fetching a Task template */ +/** A hook for fetching a Task template. TaskTemplates may have already been + * fetched as part of retrieving a Workflow. If not, we can retrieve the Task + * directly and read the template from there. + */ export function useTaskTemplate(id: Identifier): FetchableData { + const { getTask } = useAPIContext(); return useFetchableData( { + // Tasks are immutable useCache: true, debugName: 'TaskTemplate', defaultValue: {} as TaskTemplate, - // Fetching the parent workflow should insert these into the cache - // for us. If we get to this point, something went wrong. - doFetch: () => - Promise.reject( - new NotFoundError( - 'Task template', - 'No template has been loaded for this task' - ) - ) + doFetch: async taskId => + (await getTask(taskId)).closure.compiledTask.template }, id ); diff --git a/src/components/hooks/useWorkflow.ts b/src/components/hooks/useWorkflow.ts index 169abb1b9..5a9d67898 100644 --- a/src/components/hooks/useWorkflow.ts +++ b/src/components/hooks/useWorkflow.ts @@ -1,34 +1,25 @@ -import { useContext } from 'react'; - -import { CacheContext } from 'components/Cache'; import { useAPIContext } from 'components/data/apiContext'; import { Workflow, WorkflowId } from 'models'; import { FetchableData } from './types'; import { useFetchableData } from './useFetchableData'; -import { extractTaskTemplates } from './utils'; /** A hook for fetching a Workflow */ export function useWorkflow( id: WorkflowId | null = null ): FetchableData { - const cache = useContext(CacheContext); const { getWorkflow } = useAPIContext(); const doFetch = async (id: WorkflowId | null) => { if (id === null) { throw new Error('workflow id missing'); } - const workflow = await getWorkflow(id); - const templates = extractTaskTemplates(workflow); - cache.mergeArray(templates); - return workflow; + return getWorkflow(id); }; return useFetchableData( { doFetch, autoFetch: id !== null, - useCache: false, debugName: 'Workflow', defaultValue: {} as Workflow }, diff --git a/src/components/hooks/useWorkflows.ts b/src/components/hooks/useWorkflows.ts index 7bd400190..59c239082 100644 --- a/src/components/hooks/useWorkflows.ts +++ b/src/components/hooks/useWorkflows.ts @@ -12,7 +12,9 @@ import { usePagination } from './usePagination'; export function useWorkflows(scope: IdentifierScope, config: RequestConfig) { const { listWorkflows } = useAPIContext(); return usePagination( - { ...config, cacheItems: true, fetchArg: scope }, + // Workflows are not full records when listed, so don't + // cache them + { ...config, cacheItems: false, fetchArg: scope }, listWorkflows ); } diff --git a/src/components/hooks/utils.ts b/src/components/hooks/utils.ts index 7776e8d13..b734f9715 100644 --- a/src/components/hooks/utils.ts +++ b/src/components/hooks/utils.ts @@ -1,4 +1,4 @@ -import { TaskTemplate, Workflow } from 'models'; +import { GloballyUniqueNode, TaskTemplate, Workflow } from 'models'; export function extractTaskTemplates(workflow: Workflow): TaskTemplate[] { if (!workflow.closure || !workflow.closure.compiledWorkflow) { @@ -6,3 +6,32 @@ export function extractTaskTemplates(workflow: Workflow): TaskTemplate[] { } return workflow.closure.compiledWorkflow.tasks.map(t => t.template); } + +export function extractAndIdentifyNodes( + workflow: Workflow +): GloballyUniqueNode[] { + if (!workflow.closure || !workflow.closure.compiledWorkflow) { + return []; + } + const { primary, subWorkflows = [] } = workflow.closure.compiledWorkflow; + const nodes = subWorkflows.reduce( + (out, subWorkflow) => [...out, ...subWorkflow.template.nodes], + primary.template.nodes + ); + + return nodes.map(node => ({ + node, + id: { + nodeId: node.id, + // TODO: This is technically incorrect, as sub-workflow nodes + // will use the wrong parent workflow id. This is done intentionally + // to make sure that looking up the node information for a NodeExecution + // finds the entry successfully. + // When we are rendering sub-workflow nodes correctly, this should + // be updated to use the proper parent workflow id + // (subWorkflow.template.id) + // See https://github.com/lyft/flyte/issues/357 + workflowId: workflow.id + } + })); +} diff --git a/src/models/Common/types.ts b/src/models/Common/types.ts index 45b2ba67a..ee970a86b 100644 --- a/src/models/Common/types.ts +++ b/src/models/Common/types.ts @@ -35,7 +35,10 @@ export interface RetryStrategy extends Core.IRetryStrategy {} export interface RuntimeMetadata extends Core.IRuntimeMetadata {} export interface Schedule extends Admin.ISchedule {} export type MessageFormat = Core.TaskLog.MessageFormat; -export type TaskLog = RequiredNonNullable; +export interface TaskLog extends Core.ITaskLog { + name: string; + uri: string; +} /*** Literals ****/ export interface Binary extends RequiredNonNullable {} diff --git a/src/models/Execution/__mocks__/constants.ts b/src/models/Execution/__mocks__/constants.ts new file mode 100644 index 000000000..a9a29b756 --- /dev/null +++ b/src/models/Execution/__mocks__/constants.ts @@ -0,0 +1,7 @@ +import { WorkflowExecutionIdentifier } from '../types'; + +export const mockWorkflowExecutionId: WorkflowExecutionIdentifier = { + project: 'flytekit', + domain: 'development', + name: 'ABC456' +}; diff --git a/src/models/Execution/__mocks__/mockNodeExecutionsData.ts b/src/models/Execution/__mocks__/mockNodeExecutionsData.ts index 981dc443b..ed62d8858 100644 --- a/src/models/Execution/__mocks__/mockNodeExecutionsData.ts +++ b/src/models/Execution/__mocks__/mockNodeExecutionsData.ts @@ -5,15 +5,12 @@ import { CompiledNode } from 'models/Node'; import { mockNodes } from 'models/Node/__mocks__/mockNodeData'; import { NodeExecutionPhase } from '../enums'; import { NodeExecution } from '../types'; +import { mockWorkflowExecutionId } from './constants'; import { sampleError } from './sampleExecutionError'; export const mockNodeExecutionResponse: Admin.INodeExecution = { id: { - executionId: { - project: 'flytekit', - domain: 'development', - name: '4a580545ce6344fc9950' - }, + executionId: mockWorkflowExecutionId, nodeId: 'DefaultNodeId' }, inputUri: 's3://path/to/my/inputs.pb', diff --git a/src/models/Execution/__mocks__/mockTaskExecutionsData.ts b/src/models/Execution/__mocks__/mockTaskExecutionsData.ts index 568df91c6..d5da2d992 100644 --- a/src/models/Execution/__mocks__/mockTaskExecutionsData.ts +++ b/src/models/Execution/__mocks__/mockTaskExecutionsData.ts @@ -1,18 +1,38 @@ import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; -import { Admin, Core } from 'flyteidl'; +import { Admin } from 'flyteidl'; import { cloneDeep } from 'lodash'; -import { NodeExecutionPhase, TaskExecutionPhase } from '../enums'; -import { TaskExecution } from '../types'; +import { TaskLog } from 'models/Common/types'; +import { CompiledNode } from 'models/Node/types'; +import { TaskExecutionPhase } from '../enums'; +import { + NodeExecutionIdentifier, + TaskExecution, + TaskExecutionClosure +} from '../types'; import { sampleError } from './sampleExecutionError'; -const sampleLogs: Core.ITaskLog[] = [ +const sampleLogs: TaskLog[] = [ { name: 'Kubernetes Logs', uri: 'http://localhost/k8stasklog' }, { name: 'User Logs', uri: 'http://localhost/containerlog' }, { name: 'AWS Batch Logs', uri: 'http://localhost/awsbatchlog' }, { name: 'Other Custom Logs', uri: 'http://localhost/customlog' } ]; +const inputUri = 's3://path/to/my/inputs.pb'; + +function createClosure(): TaskExecutionClosure { + return { + phase: TaskExecutionPhase.SUCCEEDED, + startedAt: dateToTimestamp(new Date(Date.now() - 1000 * 60 * 10)), + createdAt: dateToTimestamp(new Date(Date.now() - 1000 * 60 * 10)), + duration: millisecondsToDuration(1000 * 60 * 60 * 1.251), + outputUri: 's3://path/to/my/outputs.pb', + logs: [...sampleLogs] + }; +} + export const mockTaskExecutionResponse: Admin.ITaskExecution = { + inputUri, id: { nodeExecutionId: { executionId: { @@ -30,19 +50,32 @@ export const mockTaskExecutionResponse: Admin.ITaskExecution = { version: 'abcdef' } }, - inputUri: 's3://path/to/my/inputs.pb', - closure: { - phase: TaskExecutionPhase.SUCCEEDED, - startedAt: dateToTimestamp(new Date(Date.now() - 1000 * 60 * 10)), - createdAt: dateToTimestamp(new Date(Date.now() - 1000 * 60 * 10)), - duration: millisecondsToDuration(1000 * 60 * 60 * 1.251), - outputUri: 's3://path/to/my/outputs.pb', - logs: [...sampleLogs] - } + closure: createClosure() }; export const mockExecution = mockTaskExecutionResponse as TaskExecution; +export function createMockTaskExecutionForNodeExecution( + nodeExecutionId: NodeExecutionIdentifier, + node: CompiledNode, + retryAttempt: number, + overrides?: Partial +): TaskExecution { + return { + ...{ + inputUri, + id: { + nodeExecutionId, + retryAttempt, + taskId: node.taskNode!.referenceId + }, + isParent: false, + closure: createClosure() + }, + ...overrides + }; +} + export const createMockTaskExecutionsListResponse = (length: number) => { return { taskExecutions: Array.from({ length }, (_, idx) => { diff --git a/src/models/Execution/__mocks__/mockWorkflowExecutionsData.ts b/src/models/Execution/__mocks__/mockWorkflowExecutionsData.ts index 105b80097..6960d4e9e 100644 --- a/src/models/Execution/__mocks__/mockWorkflowExecutionsData.ts +++ b/src/models/Execution/__mocks__/mockWorkflowExecutionsData.ts @@ -4,14 +4,11 @@ import { cloneDeep, random } from 'lodash'; import * as Long from 'long'; import { WorkflowExecutionPhase } from '../enums'; import { Execution } from '../types'; +import { mockWorkflowExecutionId } from './constants'; import { sampleError } from './sampleExecutionError'; export const mockWorkflowExecutionResponse: Admin.IExecution = { - id: { - project: 'flytekit', - domain: 'development', - name: 'ABC456' - }, + id: mockWorkflowExecutionId, spec: { launchPlan: { resourceType: Core.ResourceType.LAUNCH_PLAN, diff --git a/src/models/Node/types.ts b/src/models/Node/types.ts index b08201819..c54145227 100644 --- a/src/models/Node/types.ts +++ b/src/models/Node/types.ts @@ -45,3 +45,13 @@ export interface ConnectionSet extends Core.IConnectionSet { downstream: Record; upstream: Record; } + +export interface NodeId { + workflowId: Identifier; + nodeId: string; +} + +export interface GloballyUniqueNode { + id: NodeId; + node: CompiledNode; +} diff --git a/src/models/Workflow/types.ts b/src/models/Workflow/types.ts index 3afa905cf..59b6222ce 100644 --- a/src/models/Workflow/types.ts +++ b/src/models/Workflow/types.ts @@ -5,6 +5,7 @@ import { CompiledTask } from 'models/Task'; /** Holds information about all nodes existing in a Workflow graph */ export interface WorkflowTemplate extends Core.IWorkflowTemplate { + id: Identifier; interface?: TypedInterface; nodes: CompiledNode[]; } diff --git a/tsconfig.json b/tsconfig.json index 705b4e972..427464ed6 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,28 +1,28 @@ { - "compilerOptions": { - "allowSyntheticDefaultImports": true, - "allowJs": true, - "baseUrl": "src", - "checkJs": false, - "downlevelIteration": true, - "emitDecoratorMetadata": true, - "experimentalDecorators": true, - "importHelpers": true, - "jsx": "react", - "lib": ["es2015", "es2017", "dom", "dom.iterable"], - "module": "commonjs", - "moduleResolution": "node", - "noFallthroughCasesInSwitch": true, - "noImplicitReturns": true, - "noUnusedLocals": false, - "noUnusedParameters": false, - "outDir": "./dist", - "removeComments": false, - "resolveJsonModule": true, - "skipLibCheck": true, - "sourceMap": true, - "strict": true, - "target": "es6", - "types": ["node", "webpack-env", "jest"] - } + "compilerOptions": { + "allowSyntheticDefaultImports": true, + "allowJs": true, + "baseUrl": "src", + "checkJs": false, + "downlevelIteration": true, + "emitDecoratorMetadata": true, + "experimentalDecorators": true, + "importHelpers": true, + "jsx": "react", + "lib": ["es2015", "es2017", "dom", "dom.iterable"], + "module": "commonjs", + "moduleResolution": "node", + "noFallthroughCasesInSwitch": true, + "noImplicitReturns": true, + "noUnusedLocals": false, + "noUnusedParameters": false, + "outDir": "./dist", + "removeComments": false, + "resolveJsonModule": true, + "skipLibCheck": true, + "sourceMap": true, + "strict": true, + "target": "es6", + "types": ["node", "webpack-env", "jest"] + } }