Skip to content

Commit

Permalink
refactor: fetch child node executions for NodeExecution rows (#72)
Browse files Browse the repository at this point in the history
* refactor: fetch child node executions to determine expandability

* fix: handling detection of children for sub-workflows as well

* fix: poor performance with object-hash on some identifiers

* docs: cleanup and docs for newly exposed functions

* test: ensure request config is used for all levels

* chore: remove unused import
  • Loading branch information
schottra committed Jun 30, 2020
1 parent e11befc commit 20f273d
Show file tree
Hide file tree
Showing 10 changed files with 275 additions and 45 deletions.
3 changes: 2 additions & 1 deletion src/components/Cache/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
22 changes: 14 additions & 8 deletions src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ExecutionFilters } from '../ExecutionFilters';
import { useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState';
import { NodeExecutionsTable } from '../Tables/NodeExecutionsTable';
import { useWorkflowExecutionState } from '../useWorkflowExecutionState';
import { NodeExecutionsRequestConfigContext } from './contexts';
import { ExecutionWorkflowGraph } from './ExecutionWorkflowGraph';

const useStyles = makeStyles((theme: Theme) => ({
Expand Down Expand Up @@ -43,10 +44,11 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
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 (
<WaitForData {...workflow}>
Expand All @@ -61,10 +63,14 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
<ExecutionFilters {...filterState} />
</div>
<WaitForData {...nodeExecutions}>
<NodeExecutionsTable
{...nodeExecutions}
moreItemsAvailable={false}
/>
<NodeExecutionsRequestConfigContext.Provider
value={nodeExecutionsRequestConfig}
>
<NodeExecutionsTable
{...nodeExecutions}
moreItemsAvailable={false}
/>
</NodeExecutionsRequestConfigContext.Provider>
</WaitForData>
</>
)}
Expand Down
6 changes: 5 additions & 1 deletion src/components/Executions/ExecutionDetails/contexts.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';

import { Execution, NodeExecution } from 'models';
import { Execution, NodeExecution, RequestConfig } from 'models';

export interface ExecutionContextData {
execution: Execution;
Expand All @@ -12,3 +12,7 @@ export const ExecutionContext = React.createContext<ExecutionContextData>(
export const NodeExecutionsContext = React.createContext<
Dictionary<NodeExecution>
>({});

export const NodeExecutionsRequestConfigContext = React.createContext<
RequestConfig
>({});
17 changes: 12 additions & 5 deletions src/components/Executions/Tables/NodeExecutionRow.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as classnames from 'classnames';
import { useExpandableMonospaceTextStyles } from 'components/common/ExpandableMonospaceText';
import * as React from 'react';
import { NodeExecutionsRequestConfigContext } from '../ExecutionDetails/contexts';
import { DetailedNodeExecution } from '../types';
import { useChildNodeExecutions } from '../useChildNodeExecutions';
import { NodeExecutionsTableContext } from './contexts';
import { ExpandableExecutionError } from './ExpandableExecutionError';
import { NodeExecutionChildren } from './NodeExecutionChildren';
Expand All @@ -15,21 +17,26 @@ interface NodeExecutionRowProps {
style?: React.CSSProperties;
}

function isExpandableExecution(execution: DetailedNodeExecution) {
return true;
}

/** Renders a NodeExecution as a row inside a `NodeExecutionsTable` */
export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
columns,
execution,
style
}) => {
const state = React.useContext(NodeExecutionsTableContext);
const requestConfig = React.useContext(NodeExecutionsRequestConfigContext);

const [expanded, setExpanded] = React.useState(false);
const toggleExpanded = () => {
setExpanded(!expanded);
};

const childNodeExecutions = useChildNodeExecutions(
execution,
requestConfig
);

const isExpandable = childNodeExecutions.value.length > 0;
const tableStyles = useExecutionTableStyles();
const monospaceTextStyles = useExpandableMonospaceTextStyles();

Expand All @@ -38,7 +45,7 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
: false;
const { error } = execution.closure;

const expanderContent = isExpandableExecution(execution) ? (
const expanderContent = isExpandable ? (
<RowExpander expanded={expanded} onClick={toggleExpanded} />
) : null;

Expand Down
42 changes: 39 additions & 3 deletions src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@ import { render, waitFor } from '@testing-library/react';
import { mapNodeExecutionDetails } from 'components';
import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
import { APIContext } from 'components/data/apiContext';
import { NodeExecutionsRequestConfigContext } from 'components/Executions/ExecutionDetails/contexts';
import { DetailedNodeExecution } from 'components/Executions/types';
import { FilterOperationName, RequestConfig } from 'models';
import {
createMockWorkflow,
createMockWorkflowClosure
} from 'models/__mocks__/workflowData';
import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData';
import { listTaskExecutions } from 'models/Execution/api';
import { createMockTaskExecutionsListResponse } from 'models/Execution/__mocks__/mockTaskExecutionsData';
import {
listTaskExecutionChildren,
listTaskExecutions
} from 'models/Execution/api';
import { mockTasks } from 'models/Task/__mocks__/mockTaskData';
import * as React from 'react';
import {
Expand All @@ -18,13 +24,20 @@ import {

describe('NodeExecutionsTable', () => {
let props: NodeExecutionsTableProps;
let requestConfig: RequestConfig;
let mockNodeExecutions: DetailedNodeExecution[];
let mockListTaskExecutions: jest.Mock<ReturnType<
typeof listTaskExecutions
>>;
let mockListTaskExecutionChildren: jest.Mock<ReturnType<
typeof listTaskExecutionChildren
>>;

beforeEach(() => {
mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] });
mockListTaskExecutionChildren = jest
.fn()
.mockResolvedValue({ entities: [] });
const {
executions: mockExecutions,
nodes: mockNodes
Expand All @@ -46,6 +59,8 @@ describe('NodeExecutionsTable', () => {
mockWorkflow
);

requestConfig = {};

props = {
value: mockNodeExecutions,
lastError: null,
Expand All @@ -59,10 +74,15 @@ describe('NodeExecutionsTable', () => {
render(
<APIContext.Provider
value={mockAPIContextValue({
listTaskExecutions: mockListTaskExecutions
listTaskExecutions: mockListTaskExecutions,
listTaskExecutionChildren: mockListTaskExecutionChildren
})}
>
<NodeExecutionsTable {...props} />
<NodeExecutionsRequestConfigContext.Provider
value={requestConfig}
>
<NodeExecutionsTable {...props} />
</NodeExecutionsRequestConfigContext.Provider>
</APIContext.Provider>
);

Expand All @@ -73,4 +93,20 @@ describe('NodeExecutionsTable', () => {
const node = mockNodeExecutions[0];
expect(queryByText(node.displayId)).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).toHaveBeenCalledWith(
taskExecutions[0].id,
expect.objectContaining(requestConfig)
);
});
});
5 changes: 5 additions & 0 deletions src/components/Executions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,8 @@ export interface DetailedNodeExecution extends NodeExecution {
export interface DetailedTaskExecution extends TaskExecution {
cacheKey: string;
}

export interface NodeExecutionGroup {
name: string;
nodeExecutions: NodeExecution[];
}
142 changes: 142 additions & 0 deletions src/components/Executions/useChildNodeExecutions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { APIContextValue, useAPIContext } from 'components/data/apiContext';
import {
FetchableData,
fetchNodeExecutions,
fetchTaskExecutionChildren
} from 'components/hooks';
import { useFetchableData } from 'components/hooks/useFetchableData';
import { isEqual } from 'lodash';
import {
NodeExecution,
RequestConfig,
TaskExecutionIdentifier,
WorkflowExecutionIdentifier
} from 'models';
import * as React from 'react';
import { ExecutionContext } from './ExecutionDetails/contexts';
import { formatRetryAttempt } from './TaskExecutionsList/utils';
import { NodeExecutionGroup } from './types';
import { fetchTaskExecutions } from './useTaskExecutions';

interface FetchGroupForTaskExecutionArgs {
apiContext: APIContextValue;
config: RequestConfig;
taskExecutionId: TaskExecutionIdentifier;
}
async function fetchGroupForTaskExecution({
apiContext,
config,
taskExecutionId
}: FetchGroupForTaskExecutionArgs): Promise<NodeExecutionGroup> {
return {
// NodeExecutions created by a TaskExecution are grouped
// by the retry attempt of the task.
name: formatRetryAttempt(taskExecutionId.retryAttempt),
nodeExecutions: await fetchTaskExecutionChildren(
{ config, taskExecutionId },
apiContext
)
};
}

interface FetchGroupForWorkflowExecutionArgs {
apiContext: APIContextValue;
config: RequestConfig;
workflowExecutionId: WorkflowExecutionIdentifier;
}
async function fetchGroupForWorkflowExecution({
apiContext,
config,
workflowExecutionId
}: FetchGroupForWorkflowExecutionArgs): Promise<NodeExecutionGroup> {
return {
// NodeExecutions created by a workflow execution are grouped
// by the execution id, since workflow executions are not retryable.
name: workflowExecutionId.name,
nodeExecutions: await fetchNodeExecutions(
{ config, id: workflowExecutionId },
apiContext
)
};
}

interface FetchNodeExecutionGroupArgs {
apiContext: APIContextValue;
config: RequestConfig;
nodeExecution: NodeExecution;
}

async function fetchGroupsForTaskExecutionNode({
apiContext,
config,
nodeExecution: { id: nodeExecutionId }
}: FetchNodeExecutionGroupArgs): Promise<NodeExecutionGroup[]> {
const taskExecutions = await fetchTaskExecutions(
nodeExecutionId,
apiContext
);

return await Promise.all(
taskExecutions.map(({ id: taskExecutionId }) =>
fetchGroupForTaskExecution({ apiContext, config, taskExecutionId })
)
);
}

async function fetchGroupsForWorkflowExecutionNode({
apiContext,
config,
nodeExecution
}: FetchNodeExecutionGroupArgs): Promise<NodeExecutionGroup[]> {
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.
return [
await fetchGroupForWorkflowExecution({
apiContext,
config,
workflowExecutionId
})
];
}

/** Fetches and groups `NodeExecution`s which are direct children of the given
* `NodeExecution`.
*/
export function useChildNodeExecutions(
nodeExecution: NodeExecution,
config: RequestConfig
): FetchableData<NodeExecutionGroup[]> {
const apiContext = useAPIContext();
const { workflowNodeMetadata } = nodeExecution.closure;
const { execution: topExecution } = React.useContext(ExecutionContext);
return useFetchableData<NodeExecutionGroup[], NodeExecution>(
{
debugName: 'ChildNodeExecutions',
defaultValue: [],
doFetch: async data => {
const fetchArgs = {
apiContext,
config,
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
);
}
Loading

0 comments on commit 20f273d

Please sign in to comment.