Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: fetch child node executions for NodeExecution rows #72

Merged
merged 6 commits into from
Jun 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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