Skip to content

Commit

Permalink
feat: Expose caching status for NodeExecutions (#91)
Browse files Browse the repository at this point in the history
* chore: update flyteidl

* feat: show caching status on node executions details panel

* fix: use a different icon for cache population

* fix: adding source execution links if they exist

* fix: showing cache status in the table as well

* test: adding tests for cache status components
  • Loading branch information
schottra authored Aug 31, 2020
1 parent d1517d4 commit cb200bd
Show file tree
Hide file tree
Showing 17 changed files with 354 additions and 80 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"@commitlint/cli": "^8.3.5",
"@commitlint/config-conventional": "^8.3.4",
"@date-io/moment": "1.3.9",
"@lyft/flyteidl": "^0.17.34",
"@lyft/flyteidl": "^0.18.4",
"@material-ui/core": "^4.0.0",
"@material-ui/icons": "^4.0.0",
"@material-ui/pickers": "^3.2.2",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IconButton, Typography } from '@material-ui/core';
import { IconButton, SvgIconProps, Typography } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import Tab from '@material-ui/core/Tab';
import Tabs from '@material-ui/core/Tabs';
Expand All @@ -13,6 +13,7 @@ import { LocationDescriptor } from 'history';
import * as React from 'react';
import { Link as RouterLink } from 'react-router-dom';
import { Routes } from 'routes';
import { NodeExecutionCacheStatus } from '../NodeExecutionCacheStatus';
import { DetailedNodeExecution, NodeExecutionDisplayType } from '../types';
import { NodeExecutionInputs } from './NodeExecutionInputs';
import { NodeExecutionOutputs } from './NodeExecutionOutputs';
Expand Down Expand Up @@ -202,6 +203,9 @@ export const NodeExecutionDetails: React.FC<NodeExecutionDetailsProps> = ({
{execution.displayId}
</Typography>
{statusContent}
<NodeExecutionCacheStatus
taskNodeMetadata={execution.closure.taskNodeMetadata}
/>
<ExecutionTypeDetails execution={execution} />
</div>
</header>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
import { render, waitFor } from '@testing-library/react';
import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
import { APIContext } from 'components/data/apiContext';
import {
cacheStatusMessages,
viewSourceExecutionString
} from 'components/Executions/constants';
import {
DetailedNodeExecution,
NodeExecutionDisplayType
} from 'components/Executions/types';
import { Core } from 'flyteidl';
import { TaskNodeMetadata } from 'models';
import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData';
import { mockExecution as mockTaskExecution } from 'models/Execution/__mocks__/mockTaskExecutionsData';
import { listTaskExecutions } from 'models/Execution/api';
import * as React from 'react';
import { MemoryRouter } from 'react-router';
import { Routes } from 'routes';
import { makeIdentifier } from 'test/modelUtils';
import { NodeExecutionDetails } from '../NodeExecutionDetails';

describe('NodeExecutionDetails', () => {
Expand All @@ -28,18 +38,66 @@ describe('NodeExecutionDetails', () => {

const renderComponent = () =>
render(
<APIContext.Provider
value={mockAPIContextValue({
listTaskExecutions: mockListTaskExecutions
})}
>
<NodeExecutionDetails execution={execution} />
</APIContext.Provider>
<MemoryRouter>
<APIContext.Provider
value={mockAPIContextValue({
listTaskExecutions: mockListTaskExecutions
})}
>
<NodeExecutionDetails execution={execution} />
</APIContext.Provider>
</MemoryRouter>
);

it('renders displayId', async () => {
const { queryByText } = renderComponent();
await waitFor(() => {});
expect(queryByText(execution.displayId)).toBeInTheDocument();
});

describe('with cache information', () => {
let taskNodeMetadata: TaskNodeMetadata;
beforeEach(() => {
taskNodeMetadata = {
cacheStatus: Core.CatalogCacheStatus.CACHE_MISS,
catalogKey: {
datasetId: makeIdentifier({
resourceType: Core.ResourceType.DATASET
}),
sourceTaskExecution: { ...mockTaskExecution.id }
}
};
execution.closure.taskNodeMetadata = taskNodeMetadata;
});

[
Core.CatalogCacheStatus.CACHE_DISABLED,
Core.CatalogCacheStatus.CACHE_HIT,
Core.CatalogCacheStatus.CACHE_LOOKUP_FAILURE,
Core.CatalogCacheStatus.CACHE_MISS,
Core.CatalogCacheStatus.CACHE_POPULATED,
Core.CatalogCacheStatus.CACHE_PUT_FAILURE
].forEach(cacheStatusValue =>
it(`renders correct status for ${Core.CatalogCacheStatus[cacheStatusValue]}`, async () => {
taskNodeMetadata.cacheStatus = cacheStatusValue;
const { getByText } = renderComponent();
await waitFor(() =>
getByText(cacheStatusMessages[cacheStatusValue])
);
})
);

it('renders source execution link for cache hits', async () => {
taskNodeMetadata.cacheStatus = Core.CatalogCacheStatus.CACHE_HIT;
const sourceWorkflowExecutionId = taskNodeMetadata.catalogKey!
.sourceTaskExecution.nodeExecutionId.executionId;
const { getByText } = renderComponent();
const linkEl = await waitFor(() =>
getByText(viewSourceExecutionString)
);
expect(linkEl.getAttribute('href')).toBe(
Routes.ExecutionDetails.makeUrl(sourceWorkflowExecutionId)
);
});
});
});
129 changes: 129 additions & 0 deletions src/components/Executions/NodeExecutionCacheStatus.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { SvgIconProps, Tooltip, Typography } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import CachedOutlined from '@material-ui/icons/CachedOutlined';
import ErrorOutlined from '@material-ui/icons/ErrorOutlined';
import InfoOutlined from '@material-ui/icons/InfoOutlined';
import * as classnames from 'classnames';
import { assertNever } from 'common/utils';
import { PublishedWithChangesOutlined } from 'components/common/PublishedWithChanges';
import { useCommonStyles } from 'components/common/styles';
import { Core } from 'flyteidl';
import { TaskNodeMetadata } from 'models';
import * as React from 'react';
import { Link as RouterLink } from 'react-router-dom';
import { Routes } from 'routes';
import {
cacheStatusMessages,
unknownCacheStatusString,
viewSourceExecutionString
} from './constants';

const useStyles = makeStyles((theme: Theme) => ({
cacheStatus: {
alignItems: 'center',
display: 'flex',
marginTop: theme.spacing(1)
},
sourceExecutionLink: {
fontWeight: 'normal'
}
}));

/** Renders the appropriate icon for a given `Core.CatalogCacheStatus` */
export const NodeExecutionCacheStatusIcon: React.FC<SvgIconProps & {
status: Core.CatalogCacheStatus;
}> = React.forwardRef(({ status, ...props }, ref) => {
switch (status) {
case Core.CatalogCacheStatus.CACHE_DISABLED:
case Core.CatalogCacheStatus.CACHE_MISS: {
return <InfoOutlined {...props} ref={ref} />;
}
case Core.CatalogCacheStatus.CACHE_HIT: {
return <CachedOutlined {...props} ref={ref} />;
}
case Core.CatalogCacheStatus.CACHE_POPULATED: {
return <PublishedWithChangesOutlined {...props} ref={ref} />;
}
case Core.CatalogCacheStatus.CACHE_LOOKUP_FAILURE:
case Core.CatalogCacheStatus.CACHE_PUT_FAILURE: {
return <ErrorOutlined {...props} ref={ref} />;
}
default: {
assertNever(status);
return null;
}
}
});

export interface NodeExecutionCacheStatusProps {
taskNodeMetadata?: TaskNodeMetadata;
/** `normal` will render an icon with description message beside it
* `iconOnly` will render just the icon with the description as a tooltip
*/
variant?: 'normal' | 'iconOnly';
}
/** For a given `NodeExecution.closure.taskNodeMetadata` object, will render
* the cache status with a descriptive message. For `Core.CacheCatalogStatus.CACHE_HIT`,
* it will also attempt to render a link to the source `WorkflowExecution` (normal
* variant only).
*/
export const NodeExecutionCacheStatus: React.FC<NodeExecutionCacheStatusProps> = ({
taskNodeMetadata,
variant = 'normal'
}) => {
const commonStyles = useCommonStyles();
const styles = useStyles();
if (taskNodeMetadata == null || taskNodeMetadata.cacheStatus == null) {
return null;
}

const message =
cacheStatusMessages[taskNodeMetadata.cacheStatus] ||
unknownCacheStatusString;

const sourceExecutionId = taskNodeMetadata.catalogKey?.sourceTaskExecution;
const sourceExecutionLink = sourceExecutionId ? (
<RouterLink
className={classnames(
commonStyles.primaryLink,
styles.sourceExecutionLink
)}
to={Routes.ExecutionDetails.makeUrl(
sourceExecutionId.nodeExecutionId.executionId
)}
>
{viewSourceExecutionString}
</RouterLink>
) : null;

return variant === 'iconOnly' ? (
<Tooltip title={message} aria-label="cache status">
<NodeExecutionCacheStatusIcon
className={classnames(
commonStyles.iconLeft,
commonStyles.iconRight,
commonStyles.iconSecondary
)}
status={taskNodeMetadata.cacheStatus}
/>
</Tooltip>
) : (
<div>
<Typography
className={styles.cacheStatus}
variant="subtitle1"
color="textSecondary"
>
<NodeExecutionCacheStatusIcon
status={taskNodeMetadata.cacheStatus}
className={classnames(
commonStyles.iconSecondary,
commonStyles.iconLeft
)}
/>
{message}
</Typography>
{sourceExecutionLink}
</div>
);
};
2 changes: 1 addition & 1 deletion src/components/Executions/Tables/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ export const nodeExecutionsTableColumnWidths = {
logs: 225,
type: 144,
name: 380,
phase: 120,
phase: 150,
startedAt: 200
};
34 changes: 32 additions & 2 deletions src/components/Executions/Tables/nodeExecutionColumns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ import {
} from 'common/formatters';
import { timestampToDate } from 'common/utils';
import { useCommonStyles } from 'components/common/styles';
import { Core } from 'flyteidl';
import { TaskNodeMetadata } from 'models';
import { NodeExecutionPhase } from 'models/Execution/enums';
import * as React from 'react';
import { ExecutionStatusBadge, getNodeExecutionTimingMS } from '..';
import { NodeExecutionCacheStatus } from '../NodeExecutionCacheStatus';
import { SelectNodeExecutionLink } from './SelectNodeExecutionLink';
import { useColumnStyles } from './styles';
import {
Expand Down Expand Up @@ -44,6 +47,20 @@ const NodeExecutionName: React.FC<NodeExecutionCellRendererData> = ({
);
};

const hiddenCacheStatuses = [
Core.CatalogCacheStatus.CACHE_MISS,
Core.CatalogCacheStatus.CACHE_DISABLED
];
function hasCacheStatus(
taskNodeMetadata?: TaskNodeMetadata
): taskNodeMetadata is TaskNodeMetadata {
if (!taskNodeMetadata) {
return false;
}
const { cacheStatus } = taskNodeMetadata;
return !hiddenCacheStatuses.includes(cacheStatus);
}

export function generateColumns(
styles: ReturnType<typeof useColumnStyles>
): NodeExecutionColumnDefinition[] {
Expand All @@ -64,9 +81,22 @@ export function generateColumns(
{
cellRenderer: ({
execution: {
closure: { phase = NodeExecutionPhase.UNDEFINED }
closure: {
phase = NodeExecutionPhase.UNDEFINED,
taskNodeMetadata
}
}
}) => <ExecutionStatusBadge phase={phase} type="node" />,
}) => (
<>
<ExecutionStatusBadge phase={phase} type="node" />
{hasCacheStatus(taskNodeMetadata) ? (
<NodeExecutionCacheStatus
taskNodeMetadata={taskNodeMetadata}
variant="iconOnly"
/>
) : null}
</>
),
className: styles.columnStatus,
key: 'phase',
label: 'status'
Expand Down
1 change: 1 addition & 0 deletions src/components/Executions/Tables/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export const useColumnStyles = makeStyles((theme: Theme) => ({
flexBasis: nodeExecutionsTableColumnWidths.type
},
columnStatus: {
display: 'flex',
flexBasis: nodeExecutionsTableColumnWidths.phase
},
columnStartedAt: {
Expand Down
Loading

0 comments on commit cb200bd

Please sign in to comment.