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

[UI Feature] Add full-list log output to execution detail panel #744

Merged
merged 7 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 2 additions & 2 deletions packages/console/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@flyteorg/console",
"version": "0.0.22",
"version": "0.0.23",
"description": "Flyteconsole main app module",
"main": "./dist/index.js",
"module": "./lib/index.js",
Expand Down Expand Up @@ -59,7 +59,7 @@
"@flyteorg/common": "^0.0.4",
"@flyteorg/components": "^0.0.3",
"@flyteorg/flyte-api": "^0.0.2",
"@flyteorg/flyteidl-types": "^0.0.3",
"@flyteorg/flyteidl-types": "^0.0.4",
"@flyteorg/locale": "^0.0.2",
"@flyteorg/ui-atoms": "^0.0.3",
"@material-ui/core": "^4.12.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { NoDataIsAvailable } from 'components/Literals/LiteralMapViewer';
import { fetchWorkflow } from 'components/Workflow/workflowQueries';
import { PanelSection } from 'components/common/PanelSection';
import { DumpJSON } from 'components/common/DumpJSON';
import { ScrollableMonospaceText } from 'components/common/ScrollableMonospaceText';
import { dNode } from 'models/Graph/types';
import { NodeExecutionPhase, TaskExecutionPhase } from 'models/Execution/enums';
import {
Expand All @@ -47,7 +48,6 @@ import {
import { NodeExecutionDetails } from '../types';
import { useNodeExecutionContext } from '../contextProvider/NodeExecutionDetails';
import { getTaskExecutionDetailReasons } from './utils';
import { ExpandableMonospaceText } from '../../common/ExpandableMonospaceText';
import { fetchWorkflowExecution } from '../useWorkflowExecution';
import { NodeExecutionTabs } from './NodeExecutionTabs';
import { ExecutionDetailsActions } from './ExecutionDetailsActions';
Expand Down Expand Up @@ -470,10 +470,7 @@ export const NodeExecutionDetailsPanelContent: React.FC<
</div>
{isRunningPhase && isReasonsVisible && (
<div className={styles.statusBody}>
<ExpandableMonospaceText
initialExpansionState={false}
text={reasons.join('\n')}
/>
<ScrollableMonospaceText text={reasons.join('\n\n')} />
</div>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
} from 'models/Execution/types';
import { Routes } from 'routes/routes';
import { PaginatedEntityResponse } from 'models/AdminEntity/types';
import { compareTimestampsAscending, timestampToDate } from 'common/utils';
import { formatDateUTC } from 'common/formatters';

export function isSingleTaskExecution(execution: Execution) {
return execution.spec.launchPlan.resourceType === ResourceType.TASK;
Expand All @@ -27,10 +29,22 @@ export function getExecutionBackLink(execution: Execution): string {
export function getTaskExecutionDetailReasons(
taskExecutionDetails?: PaginatedEntityResponse<TaskExecution>,
): (string | null | undefined)[] {
return (
const reasons = (
taskExecutionDetails?.entities.map(
taskExecution => taskExecution.closure.reason,
taskExecution => taskExecution.closure.reasons || [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add a fallback here for cases where users are running older versions of admin; fall back to reason.

) || []
)
.flat()
.sort((a, b) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also don't want to do sorting here; just print the array as returned.

if (!a || !a.occurredAt) return -1;
if (!b || !b.occurredAt) return 1;
return compareTimestampsAscending(a.occurredAt, b.occurredAt);
});
return reasons.map(
reason =>
(reason.occurredAt
? formatDateUTC(timestampToDate(reason.occurredAt)) + '\n'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to format the date; just show the text as-is here.

: '') + reason.message,
);
}

Expand Down
119 changes: 119 additions & 0 deletions packages/console/src/components/common/ScrollableMonospaceText.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import { Button } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import FileCopy from '@material-ui/icons/FileCopy';
import classnames from 'classnames';
import {
errorBackgroundColor,
listhoverColor,
nestedListColor,
primaryHighlightColor,
separatorColor,
} from 'components/Theme/constants';
import copyToClipboard from 'copy-to-clipboard';
import * as React from 'react';

const showOnHoverClass = 'showOnHover';

export const useScrollableMonospaceTextStyles = makeStyles((theme: Theme) => ({
actionContainer: {
transition: theme.transitions.create('opacity', {
duration: theme.transitions.duration.shorter,
easing: theme.transitions.easing.easeInOut,
}),
},
wrapper: {
position: 'relative',
},
container: {
backgroundColor: errorBackgroundColor,
border: `1px solid ${separatorColor}`,
borderRadius: 4,
height: theme.spacing(12),
minHeight: theme.spacing(12),
overflowY: 'scroll',
padding: theme.spacing(2),
display: 'flex',
flexDirection: 'column-reverse',
'$nestedParent &': {
backgroundColor: theme.palette.common.white,
},
// All children using the showOnHover class will be hidden until
// the mouse enters the container
[`& .${showOnHoverClass}`]: {
opacity: 0,
},
[`&:hover .${showOnHoverClass}`]: {
opacity: 1,
},
},
copyButton: {
backgroundColor: theme.palette.common.white,
border: `1px solid ${primaryHighlightColor}`,
borderRadius: theme.spacing(1),
color: theme.palette.text.secondary,
height: theme.spacing(4),
minWidth: 0,
padding: 0,
position: 'absolute',
right: theme.spacing(3),
top: theme.spacing(1),
width: theme.spacing(4),
'&:hover': {
backgroundColor: listhoverColor,
},
},
errorMessage: {
fontFamily: 'monospace',
whiteSpace: 'pre-wrap',
wordBreak: 'break-all',
wordWrap: 'break-word',
},
// Apply this class to an ancestor container to have the expandable text
// use an alternate background color scheme.
nestedParent: {
backgroundColor: nestedListColor,
},
}));

export interface ScrollableMonospaceTextProps {
text: string;
}

/** An expandable/collapsible container which renders the provided text in a
* monospace font. It also provides a button to copy the text.
*/
export const ScrollableMonospaceText: React.FC<
ScrollableMonospaceTextProps
> = ({ text }) => {
const styles = useScrollableMonospaceTextStyles();
const scrollRef = React.useRef<HTMLDivElement>(null);
const onClickCopy = (e: React.MouseEvent<HTMLElement>) => {
// prevent the parent row body onClick event trigger
e.stopPropagation();
copyToClipboard(text);
};

React.useEffect(() => {
scrollRef.current?.scrollTo({
top: scrollRef.current.scrollHeight,
});
}, [text, scrollRef.current]);

return (
<div className={classnames(styles.wrapper)}>
<div className={classnames(styles.container)} ref={scrollRef}>
<div className={styles.errorMessage}>{`${text}`}</div>
<div className={classnames(styles.actionContainer, showOnHoverClass)}>
<Button
color="primary"
className={styles.copyButton}
onClick={onClickCopy}
variant="outlined"
>
<FileCopy fontSize="small" />
</Button>
</div>
</div>
</div>
);
};
1 change: 1 addition & 0 deletions packages/console/src/models/Execution/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export interface TaskExecutionClosure extends Admin.ITaskExecutionClosure {
phase: TaskExecutionPhase;
startedAt?: Protobuf.ITimestamp;
eventVersion?: number;
reasons?: Admin.IReason[];
}

/** Execution data */
Expand Down
4 changes: 2 additions & 2 deletions packages/flyteidl-types/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@flyteorg/flyteidl-types",
"version": "0.0.3",
"version": "0.0.4",
"description": "Flyteconsole basics flyteidl types",
"main": "./dist/index.js",
"module": "./lib/index.js",
Expand Down Expand Up @@ -46,6 +46,6 @@
"protobufjs": "~6.11.3"
},
"dependencies": {
"@flyteorg/flyteidl": "1.3.4"
"@flyteorg/flyteidl": "1.3.18"
}
}
2 changes: 1 addition & 1 deletion website/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"dependencies": {
"@flyteorg/common": "^0.0.4",
"@flyteorg/console": "^0.0.22",
"@flyteorg/console": "^0.0.23",
"long": "^4.0.0",
"protobufjs": "~6.11.3",
"react-ga4": "^1.4.1",
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1995,7 +1995,7 @@ __metadata:
resolution: "@flyteconsole/client-app@workspace:website"
dependencies:
"@flyteorg/common": ^0.0.4
"@flyteorg/console": ^0.0.22
"@flyteorg/console": ^0.0.23
"@types/long": ^3.0.32
long: ^4.0.0
protobufjs: ~6.11.3
Expand Down Expand Up @@ -2034,7 +2034,7 @@ __metadata:
languageName: unknown
linkType: soft

"@flyteorg/console@^0.0.22, @flyteorg/console@workspace:packages/console":
"@flyteorg/console@^0.0.23, @flyteorg/console@workspace:packages/console":
version: 0.0.0-use.local
resolution: "@flyteorg/console@workspace:packages/console"
dependencies:
Expand All @@ -2043,7 +2043,7 @@ __metadata:
"@flyteorg/common": ^0.0.4
"@flyteorg/components": ^0.0.3
"@flyteorg/flyte-api": ^0.0.2
"@flyteorg/flyteidl-types": ^0.0.3
"@flyteorg/flyteidl-types": ^0.0.4
"@flyteorg/locale": ^0.0.2
"@flyteorg/ui-atoms": ^0.0.3
"@material-ui/core": ^4.12.4
Expand Down Expand Up @@ -2137,20 +2137,20 @@ __metadata:
languageName: unknown
linkType: soft

"@flyteorg/flyteidl-types@^0.0.3, @flyteorg/flyteidl-types@workspace:packages/flyteidl-types":
"@flyteorg/flyteidl-types@^0.0.4, @flyteorg/flyteidl-types@workspace:packages/flyteidl-types":
version: 0.0.0-use.local
resolution: "@flyteorg/flyteidl-types@workspace:packages/flyteidl-types"
dependencies:
"@flyteorg/flyteidl": 1.3.4
"@flyteorg/flyteidl": 1.3.18
peerDependencies:
protobufjs: ~6.11.3
languageName: unknown
linkType: soft

"@flyteorg/flyteidl@npm:1.3.4":
version: 1.3.4
resolution: "@flyteorg/flyteidl@npm:1.3.4"
checksum: 8674f2dce839e544861f728c4b617fcc61f1ce9d9f37711d211ee9a000766cf84e9c7703f0fb7ffdbb7f66a5617bbb250f66d9099b4ad4c220532ec9ccab88ac
"@flyteorg/flyteidl@npm:1.3.18":
version: 1.3.18
resolution: "@flyteorg/flyteidl@npm:1.3.18"
checksum: f0c16a8f0eb3f1ab79ca9e4c1c871bf5a8c848a81d021bd3ec9d4daf7869e99d09f94a2feb266da0e2b7694c9d97a9386604df630b9fe5a76ba0c15a00099a6f
languageName: node
linkType: hard

Expand Down