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

feat: Expose caching status for NodeExecutions #91

Merged
merged 6 commits into from
Aug 31, 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
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