Skip to content

Commit

Permalink
chore: fix TaskType display to ensure consistency, code cleanup, add …
Browse files Browse the repository at this point in the history
…mapTaskListItem component (flyteorg#338)

* chore: add component for mapped task support
* test: add test coverage for isMapTaskType and MapTaskStatusInfo
* chore: ignore *.stories.tsx files when collecting coverage
Signed-off-by: Nastya Rusina <[email protected]>
  • Loading branch information
anrusina authored Mar 23, 2022
1 parent e00127a commit 3c7f27c
Show file tree
Hide file tree
Showing 30 changed files with 277 additions and 60 deletions.
5 changes: 5 additions & 0 deletions .storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ export const parameters = {
date: /Date$/,
},
},
options: {
storySort: {
order: ['Primitives', 'Common'],
},
},
};

export const decorators = [
Expand Down
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = {
'<rootDir>/assetsTransformer.js',
},
coverageDirectory: '.coverage',
collectCoverageFrom: ['**/*.{ts,tsx}'],
collectCoverageFrom: ['**/*.{ts,tsx}', '!**/*/*.stories.tsx'],
coveragePathIgnorePatterns: [
'__stories__',
'<rootDir>/.storybook',
Expand Down
2 changes: 1 addition & 1 deletion src/components/Errors/__stories__/DataError.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { DataError } from '../DataError';

const retryAction = action('retry');

const stories = storiesOf('Errors/DataError', module);
const stories = storiesOf('Common/DataError', module);
stories.add('Title Only', () => (
<DataError errorTitle="Something went wrong" retry={retryAction} />
));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import * as React from 'react';
import { Tab, Tabs } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import { noop } from 'lodash';

import { FeatureFlag, useFeatureFlag } from 'basics/FeatureFlags';
import { WaitForQuery } from 'components/common/WaitForQuery';
import { DataError } from 'components/Errors/DataError';
import { useTabState } from 'components/hooks/useTabState';
import { secondaryBackgroundColor } from 'components/Theme/constants';
import { Execution, NodeExecution } from 'models/Execution/types';
import { LocalCacheItem, useLocalCache } from 'basics/LocalCache';
import { NodeExecutionDetailsContextProvider } from '../contextProvider/NodeExecutionDetails';
import { NodeExecutionsRequestConfigContext } from '../contexts';
import { ExecutionFilters } from '../ExecutionFilters';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { makeStyles, Theme } from '@material-ui/core/styles';
import { storiesOf } from '@storybook/react';
import { NodeExecutionDetailsContext } from 'components/Executions/contextProvider/NodeExecutionDetails';
import { makeNodeExecutionListQuery } from 'components/Executions/nodeExecutionQueries';
import { NodeExecutionDisplayType } from 'components/Executions/types';
import { basicPythonWorkflow } from 'mocks/data/fixtures/basicPythonWorkflow';
import * as React from 'react';
import { useQuery, useQueryClient } from 'react-query';
Expand All @@ -26,7 +27,7 @@ const getNodeExecutionDetails = async () => {
return {
displayId: 'node0',
displayName: 'basic.byton.workflow.unique.task_name',
displayType: 'Python-Task',
displayType: NodeExecutionDisplayType.PythonTask,
};
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import * as React from 'react';
import { Typography } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import { NewTargetLink } from 'components/common/NewTargetLink';
import { useCommonStyles } from 'components/common/styles';
import { TaskLog } from 'models/Common/types';
import { TaskExecution } from 'models/Execution/types';
import * as React from 'react';
import { noLogsFoundString } from '../constants';

const useStyles = makeStyles((theme: Theme) => ({
Expand Down Expand Up @@ -33,19 +32,17 @@ const TaskLogList: React.FC<{ logs: TaskLog[] }> = ({ logs }) => {
);
};

/** Renders log links from a `TaskExecution`, if they exist. Otherwise renders
* a message indicating that no logs are available.
/** Renders log links from a `taskLogs`(aka taskExecution.closure.logs), if they exist.
* Otherwise renders a message indicating that no logs are available.
*/
export const TaskExecutionLogs: React.FC<{ taskExecution: TaskExecution }> = ({
taskExecution,
}) => {
export const TaskExecutionLogs: React.FC<{ taskLogs: TaskLog[] }> = ({ taskLogs }) => {
const styles = useStyles();
return (
<section>
<header className={styles.sectionHeader}>
<Typography variant="h6">Logs</Typography>
</header>
<TaskLogList logs={taskExecution.closure.logs || []} />
<TaskLogList logs={taskLogs} />
</section>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const TaskExecutionsListItem: React.FC<TaskExecutionsListItemProps> = ({
{taskHasStarted && (
<>
<section className={styles.section}>
<TaskExecutionLogs taskExecution={taskExecution} />
<TaskExecutionLogs taskLogs={taskExecution.closure.logs || []} />
</section>
<section className={styles.section}>
<TaskExecutionDetails taskExecution={taskExecution} />
Expand Down
4 changes: 3 additions & 1 deletion src/components/Executions/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ export const taskExecutionPhaseConstants: {
export const taskTypeToNodeExecutionDisplayType: {
[k in TaskType]: NodeExecutionDisplayType;
} = {
[TaskType.ARRAY]: NodeExecutionDisplayType.ArrayTask,
[TaskType.ARRAY]: NodeExecutionDisplayType.MapTask,
[TaskType.BATCH_HIVE]: NodeExecutionDisplayType.BatchHiveTask,
[TaskType.DYNAMIC]: NodeExecutionDisplayType.DynamicTask,
[TaskType.HIVE]: NodeExecutionDisplayType.HiveTask,
Expand All @@ -192,6 +192,8 @@ export const taskTypeToNodeExecutionDisplayType: {
[TaskType.UNKNOWN]: NodeExecutionDisplayType.UnknownTask,
[TaskType.WAITABLE]: NodeExecutionDisplayType.WaitableTask,
[TaskType.MPI]: NodeExecutionDisplayType.MpiTask,
[TaskType.ARRAY_AWS]: NodeExecutionDisplayType.ARRAY_AWS,
[TaskType.ARRAY_K8S]: NodeExecutionDisplayType.ARRAY_K8S,
};

export const cacheStatusMessages: { [k in Core.CatalogCacheStatus]: string } = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { transformerWorkflowToDag } from 'components/WorkflowGraph/transformerWorkflowToDag';
import { getTaskDisplayType } from 'components/Executions/utils';
import { NodeExecutionDetails, NodeExecutionDisplayType } from 'components/Executions/types';
import { Workflow } from 'models/Workflow/types';
import { Identifier } from 'models/Common/types';
Expand Down Expand Up @@ -37,11 +38,13 @@ const getNodeDetails = (node: dNode, tasks: CompiledTask[]): NodeExecutionInfo =
if (node.value.taskNode) {
const templateName = node.value.taskNode.referenceId.name ?? node.name;
const task = tasks.find((t) => t.template.id.name === templateName);
const taskType = getTaskDisplayType(task?.template.type);

return {
scopedId: node.scopedId,
displayId: node.value.id ?? node.id,
displayName: templateName,
displayType: task?.template.type ?? NodeExecutionDisplayType.UnknownTask,
displayType: taskType,
taskTemplate: task?.template,
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getTaskDisplayType } from 'components/Executions/utils';
import { fetchTaskExecutionList } from 'components/Executions/taskExecutionQueries';
import { NodeExecutionDetails, NodeExecutionDisplayType } from 'components/Executions/types';
import { NodeExecutionDetails } from 'components/Executions/types';
import { fetchTaskTemplate } from 'components/Task/taskQueries';
import { NodeExecution } from 'models/Execution/types';
import { TaskTemplate } from 'models/Task/types';
Expand All @@ -15,6 +16,7 @@ export const getTaskThroughExecution = async (
if (taskExecutions && taskExecutions.length > 0) {
taskTemplate = await fetchTaskTemplate(queryClient, taskExecutions[0].id.taskId);
if (!taskTemplate) {
// eslint-disable-next-line no-console
console.error(
`ERROR: Unexpected missing task template while fetching NodeExecution details: ${JSON.stringify(
taskExecutions[0].id.taskId,
Expand All @@ -26,7 +28,7 @@ export const getTaskThroughExecution = async (
const taskDetails: NodeExecutionDetails = {
displayId: nodeExecution.id.nodeId,
displayName: taskExecutions?.[0]?.id.taskId.name,
displayType: taskTemplate?.type ?? NodeExecutionDisplayType.Unknown,
displayType: getTaskDisplayType(taskTemplate?.type),
taskTemplate: taskTemplate,
};

Expand Down
5 changes: 4 additions & 1 deletion src/components/Executions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface ExecutionPhaseConstants {
}

export enum NodeExecutionDisplayType {
ArrayTask = 'Array Task',
MapTask = 'Map Task',
BatchHiveTask = 'Hive Batch Task',
BranchNode = 'Branch Node',
DynamicTask = 'Dynamic Task',
Expand All @@ -26,6 +26,9 @@ export enum NodeExecutionDisplayType {
UnknownTask = 'Unknown Task',
WaitableTask = 'Waitable Task',
MpiTask = 'MPI Task',
// plugins
ARRAY_AWS = 'AWS Map Task',
ARRAY_K8S = 'K8S Map Task',
}

export interface ParentNodeExecution extends NodeExecution {
Expand Down
14 changes: 13 additions & 1 deletion src/components/Executions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import { CompiledNode } from 'models/Node/types';
import {
nodeExecutionPhaseConstants,
taskExecutionPhaseConstants,
taskTypeToNodeExecutionDisplayType,
workflowExecutionPhaseConstants,
} from './constants';
import { ExecutionPhaseConstants, ParentNodeExecution } from './types';
import { ExecutionPhaseConstants, NodeExecutionDisplayType, ParentNodeExecution } from './types';

/** Given an execution phase, returns a set of constants (i.e. color, display
* string) used to represent it in various UI components.
Expand Down Expand Up @@ -55,6 +56,17 @@ export function getTaskExecutionPhaseConstants(phase: TaskExecutionPhase): Execu
);
}

/**
* Transforms taskType value to more convinient readable display Type
*/
export function getTaskDisplayType(taskType?: string): string {
if (taskType) {
return taskTypeToNodeExecutionDisplayType[taskType] ?? taskType;
}

return NodeExecutionDisplayType.Unknown;
}

/** Determines if a workflow execution can be considered finalized and will not
* change state again.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const renderBanner = (status: SystemStatus) => (
<RenderSystemStatusBanner systemStatus={status} onClose={action('onClose')} />
);

const stories = storiesOf('Notifications/SystemStatusBanner', module);
const stories = storiesOf('Common/SystemStatusBanner', module);
stories.add('Normal Status', () => renderBanner(normalStatus));
stories.add('Degraded Status', () => renderBanner(degradedStatus));
stories.add('Down Status', () => renderBanner(downStatus));
Expand Down
11 changes: 7 additions & 4 deletions src/components/Theme/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,19 @@ export const statusColors = {

export type TaskColorMap = Record<TaskType, string>;
export const taskColors: TaskColorMap = {
[TaskType.PYTHON]: '#7157D9',
[TaskType.SPARK]: '#00B3A4',
[TaskType.MPI]: '#00B3A4',
[TaskType.BATCH_HIVE]: '#E1E8ED',
[TaskType.DYNAMIC]: '#E1E8ED',
[TaskType.HIVE]: '#E1E8ED',
[TaskType.PYTHON]: '#7157D9',
[TaskType.SPARK]: '#00B3A4',
[TaskType.ARRAY]: '#E1E8ED',
[TaskType.SIDECAR]: '#E1E8ED',
[TaskType.UNKNOWN]: '#E1E8ED',
[TaskType.WAITABLE]: '#E1E8ED',
[TaskType.MPI]: '#00B3A4',
[TaskType.ARRAY]: '#E1E8ED',
// plugins
[TaskType.ARRAY_AWS]: '#E1E8ED',
[TaskType.ARRAY_K8S]: '#E1E8ED',
};

export const bodyFontSize = '0.875rem';
Expand Down
2 changes: 1 addition & 1 deletion src/components/WorkflowGraph/WorkflowGraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { NodeExecutionsContext } from 'components/Executions/contexts';
import { WaitForQuery } from 'components/common/WaitForQuery';
import { useQuery, useQueryClient } from 'react-query';
import { makeNodeExecutionDynamicWorkflowQuery } from 'components/Workflow/workflowQueries';
import { createDebugLogger } from 'components/flytegraph/utils';
import { createDebugLogger } from 'common/log';
import { CompiledNode } from 'models/Node/types';
import { transformerWorkflowToDag } from './transformerWorkflowToDag';

Expand Down
2 changes: 1 addition & 1 deletion src/components/WorkflowGraph/transformerWorkflowToDag.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DISPLAY_NAME_END, DISPLAY_NAME_START } from 'components/flytegraph/ReactFlow/utils';
import { createDebugLogger } from 'components/flytegraph/utils';
import { createDebugLogger } from 'common/log';
import { dTypes, dEdge, dNode } from 'models/Graph/types';
import { startNodeId, endNodeId } from 'models/Node/constants';
import { CompiledNode, ConnectionSet, TaskNode } from 'models/Node/types';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import * as React from 'react';
import { ComponentStory, ComponentMeta } from '@storybook/react';
import { NodeExecutionPhase } from 'models/Execution/enums';
import { MapTaskStatusInfo } from './MapTaskStatusInfo';
import { PanelViewDecorator } from '../__stories__/Decorators';

export default {
title: 'Common/MapTaskExecutionList/MapTaskStatusInfo',
component: MapTaskStatusInfo,
parameters: { actions: { argTypesRegex: 'toggleExpanded' } },
} as ComponentMeta<typeof MapTaskStatusInfo>;

const Template: ComponentStory<typeof MapTaskStatusInfo> = (args) => (
<MapTaskStatusInfo {...args} />
);

export const Default = Template.bind({});
Default.decorators = [(Story) => PanelViewDecorator(Story)];
Default.args = {
taskLogs: [
{ uri: '#', name: 'Kubernetes Logs #0-0' },
{ uri: '#', name: 'Kubernetes Logs #0-1' },
{ uri: '#', name: 'Kubernetes Logs #0-2' },
{ uri: '#', name: 'Kubernetes Logs #0-3' },
{ uri: '#', name: 'Kubernetes Logs #0-4' },
],
status: NodeExecutionPhase.QUEUED,
expanded: true,
};

export const AllSpace = Template.bind({});
AllSpace.args = {
taskLogs: [],
status: NodeExecutionPhase.SUCCEEDED,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { fireEvent, render, waitFor } from '@testing-library/react';
import { noLogsFoundString } from 'components/Executions/constants';
import { getNodeExecutionPhaseConstants } from 'components/Executions/utils';
import { NodeExecutionPhase } from 'models/Execution/enums';
import * as React from 'react';

import { MapTaskStatusInfo } from './MapTaskStatusInfo';

const taskLogs = [
{ uri: '#', name: 'Kubernetes Logs #0-0' },
{ uri: '#', name: 'Kubernetes Logs #0-1' },
{ uri: '#', name: 'Kubernetes Logs #0-2' },
];

describe('MapTaskStatusInfo', () => {
it('Phase and amount of links rendered correctly', async () => {
const status = NodeExecutionPhase.RUNNING;
const phaseData = getNodeExecutionPhaseConstants(status);

const { queryByText, getByTitle } = render(
<MapTaskStatusInfo taskLogs={taskLogs} status={status} expanded={false} />,
);

expect(queryByText(phaseData.text)).toBeInTheDocument();
expect(queryByText(`x${taskLogs.length}`)).toBeInTheDocument();
expect(queryByText('Logs')).not.toBeInTheDocument();

// Expand item - see logs section
const buttonEl = getByTitle('Expand row');
fireEvent.click(buttonEl);
await waitFor(() => {
expect(queryByText('Logs')).toBeInTheDocument();
});
});

it('Phase with no links show proper texts when opened', () => {
const status = NodeExecutionPhase.ABORTED;
const phaseData = getNodeExecutionPhaseConstants(status);

const { queryByText } = render(
<MapTaskStatusInfo taskLogs={[]} status={status} expanded={true} />,
);

expect(queryByText(phaseData.text)).toBeInTheDocument();
expect(queryByText(`x0`)).toBeInTheDocument();
expect(queryByText(noLogsFoundString)).toBeInTheDocument();
});
});
Loading

0 comments on commit 3c7f27c

Please sign in to comment.