From d43a1e393ef7b0651f9774460e3d7166a3b57f17 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 1 Jul 2020 12:53:45 -0700 Subject: [PATCH] fix: Expose cluster information for Workflow Executions (#78) * chore: update flyteidl * fix: move some of execution content out of page header * fix: show cluster name if available * refactor: rearranging metadata display and adding cluster info * refactor: pr feedback * fix: also show execution errors when available * fix: makes execution metadata collapsible * fix: don't show execution metadata without values --- package.json | 2 +- .../ExecutionDetails/ExecutionDetails.tsx | 49 +++++++ .../ExecutionDetailsAppBarContent.tsx | 99 +++----------- .../ExecutionDetails/ExecutionMetadata.tsx | 124 ++++++++++++++++++ .../ExecutionDetails/ExecutionNodeViews.tsx | 2 + .../Executions/ExecutionDetails/constants.ts | 7 + .../test/ExecutionMetadata.test.tsx | 58 ++++++++ src/components/Theme/constants.ts | 1 + src/models/__mocks__/executionsData.ts | 5 +- yarn.lock | 33 ++--- 10 files changed, 274 insertions(+), 106 deletions(-) create mode 100644 src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx create mode 100644 src/components/Executions/ExecutionDetails/constants.ts create mode 100644 src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx diff --git a/package.json b/package.json index a13390c82..2c1b86cc5 100644 --- a/package.json +++ b/package.json @@ -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.27", + "@lyft/flyteidl": "^0.17.34", "@material-ui/core": "^4.0.0", "@material-ui/icons": "^4.0.0", "@material-ui/pickers": "^3.2.2", diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx index ecf511921..8360c7c95 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx @@ -1,3 +1,7 @@ +import { Collapse, IconButton } from '@material-ui/core'; +import { makeStyles, Theme } from '@material-ui/core/styles'; +import ExpandMore from '@material-ui/icons/ExpandMore'; +import * as classnames from 'classnames'; import { WaitForData, withRouteParams } from 'components/common'; import { RefreshConfig, useDataRefresher } from 'components/hooks'; import { Execution } from 'models'; @@ -8,8 +12,32 @@ import { useExecutionDataCache } from '../useExecutionDataCache'; import { useWorkflowExecution } from '../useWorkflowExecution'; import { executionIsTerminal } from '../utils'; import { ExecutionDetailsAppBarContent } from './ExecutionDetailsAppBarContent'; +import { ExecutionMetadata } from './ExecutionMetadata'; import { ExecutionNodeViews } from './ExecutionNodeViews'; +const useStyles = makeStyles((theme: Theme) => ({ + expandCollapseButton: { + transition: theme.transitions.create('transform'), + '&.expanded': { + transform: 'rotate(180deg)' + } + }, + expandCollapseContainer: { + alignItems: 'center', + bottom: 0, + display: 'flex', + // Matches height of tabs in the NodeViews container + height: theme.spacing(6), + position: 'absolute', + right: theme.spacing(3), + transform: 'translateY(100%)', + zIndex: 1 + }, + metadataContainer: { + position: 'relative' + } +})); + export interface ExecutionDetailsRouteParams { domainId: string; executionId: string; @@ -33,6 +61,9 @@ export const ExecutionDetailsContainer: React.FC = domain: domainId, name: executionId }; + const styles = useStyles(); + const [metadataExpanded, setMetadataExpanded] = React.useState(true); + const toggleMetadata = () => setMetadataExpanded(!metadataExpanded); const dataCache = useExecutionDataCache(); const { fetchable, terminateExecution } = useWorkflowExecution( id, @@ -43,10 +74,28 @@ export const ExecutionDetailsContainer: React.FC = terminateExecution, execution: fetchable.value }; + return ( +
+ + + +
+ + + +
+
diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetailsAppBarContent.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetailsAppBarContent.tsx index 0591ece20..696bcb496 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionDetailsAppBarContent.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionDetailsAppBarContent.tsx @@ -2,8 +2,7 @@ import { Button, Dialog, Link, Typography } from '@material-ui/core'; import { makeStyles, Theme } from '@material-ui/core/styles'; import ArrowBack from '@material-ui/icons/ArrowBack'; import * as classnames from 'classnames'; -import { formatDateUTC, protobufDurationToHMS } from 'common/formatters'; -import { timestampToDate } from 'common/utils'; +import { navbarGridHeight } from 'common/layout'; import { useCommonStyles } from 'components/common/styles'; import { useLocationState } from 'components/hooks/useLocationState'; import { NavBarContent } from 'components/Navigation/NavBarContent'; @@ -19,9 +18,6 @@ import { executionIsTerminal } from '../utils'; import { RelaunchExecutionForm } from './RelaunchExecutionForm'; const useStyles = makeStyles((theme: Theme) => { - const actionsMinWidth = theme.spacing(34); - const badgeWidth = theme.spacing(11); - const maxDetailsWidth = `calc(100% - ${actionsMinWidth + badgeWidth}px)`; return { actions: { alignItems: 'center', @@ -41,25 +37,13 @@ const useStyles = makeStyles((theme: Theme) => { flex: '1 1 auto', maxWidth: '100%' }, - detailsContainer: { + titleContainer: { alignItems: 'center', display: 'flex', flex: '0 1 auto', - justifyContent: 'space-between', - maxWidth: maxDetailsWidth - }, - detailItem: { - flexShrink: 0, - marginLeft: theme.spacing(2) - }, - detailLabel: { - fontSize: smallFontSize, - lineHeight: 1.25 - }, - detailValue: { - fontSize: '0.875rem', - fontWeight: 'bold', - lineHeight: '1.1875rem' + flexDirection: 'column', + maxHeight: theme.spacing(navbarGridHeight), + overflow: 'hidden' }, inputsOutputsLink: { color: interactiveTextDisabledColor @@ -69,7 +53,7 @@ const useStyles = makeStyles((theme: Theme) => { }, title: { flex: '0 1 auto', - overflow: 'hidden' + marginLeft: theme.spacing(2) }, version: { flex: '0 1 auto', @@ -78,12 +62,6 @@ const useStyles = makeStyles((theme: Theme) => { }; }); -interface DetailItem { - className?: string; - label: React.ReactNode; - value: React.ReactNode; -} - /** Renders information about a given Execution into the NavBar */ export const ExecutionDetailsAppBarContent: React.FC<{ execution: Execution; @@ -94,7 +72,7 @@ export const ExecutionDetailsAppBarContent: React.FC<{ const [showRelaunchForm, setShowRelaunchForm] = React.useState(false); const { domain, name, project } = execution.id; - const { duration, startedAt, phase, workflowId } = execution.closure; + const { phase, workflowId } = execution.closure; const { backLink = Routes.WorkflowDetails.makeUrl( @@ -135,28 +113,6 @@ export const ExecutionDetailsAppBarContent: React.FC<{ ); - const details: DetailItem[] = [ - { - className: styles.title, - label: `${project}/${domain}/${workflowId.name}`, - value: name - }, - { label: 'Domain', value: domain }, - { - className: styles.version, - label: 'Version', - value: workflowId.version - }, - { - label: 'Time', - value: startedAt ? formatDateUTC(timestampToDate(startedAt)) : '' - }, - { - label: 'Duration', - value: duration ? protobufDurationToHMS(duration) : '' - } - ]; - return ( <> @@ -165,34 +121,19 @@ export const ExecutionDetailsAppBarContent: React.FC<{ -
- {details.map(({ className, label, value }, idx) => ( -
- - {label} - -
- {value} -
-
- ))} +
+ + + {`${project}/${domain}/${workflowId.name}/`} + {`${name}`} + +
{ + return { + container: { + background: secondaryBackgroundColor, + display: 'flex', + flexDirection: 'column', + position: 'relative' + }, + detailsContainer: { + alignItems: 'center', + display: 'flex', + flex: '0 1 auto', + paddingTop: theme.spacing(3), + paddingBottom: theme.spacing(2) + }, + detailItem: { + flexShrink: 0, + marginLeft: theme.spacing(4) + }, + expandCollapseButton: { + transition: theme.transitions.create('transform'), + '&.expanded': { + transform: 'rotate(180deg)' + } + }, + expandCollapseContainer: { + bottom: 0, + position: 'absolute', + right: theme.spacing(2), + transform: 'translateY(100%)', + zIndex: 1 + }, + version: { + flex: '0 1 auto', + overflow: 'hidden' + } + }; +}); + +interface DetailItem { + className?: string; + label: React.ReactNode; + value: React.ReactNode; +} + +/** Renders metadata details about a given Execution */ +export const ExecutionMetadata: React.FC<{ + execution: Execution; +}> = ({ execution }) => { + const commonStyles = useCommonStyles(); + const styles = useStyles(); + + const { domain } = execution.id; + const { duration, error, startedAt, workflowId } = execution.closure; + const { systemMetadata } = execution.spec.metadata; + const cluster = systemMetadata?.executionCluster ?? unknownValueString; + + const details: DetailItem[] = [ + { label: ExecutionMetadataLabels.domain, value: domain }, + { + className: styles.version, + label: ExecutionMetadataLabels.version, + value: workflowId.version + }, + { + label: ExecutionMetadataLabels.cluster, + value: cluster + } + ]; + if (startedAt) { + details.push({ + label: ExecutionMetadataLabels.time, + value: formatDateUTC(timestampToDate(startedAt)) + }); + } + if (duration) { + details.push({ + label: ExecutionMetadataLabels.duration, + value: protobufDurationToHMS(duration) + }); + } + + return ( +
+
+ {details.map(({ className, label, value }, idx) => ( +
+ + {label} + + + {value} + +
+ ))} +
+ + {error ? : null} +
+ ); +}; diff --git a/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx b/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx index d5d39d4e4..e2d457fa3 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx @@ -2,6 +2,7 @@ import { Tab, Tabs } from '@material-ui/core'; import { makeStyles, Theme } from '@material-ui/core/styles'; import { WaitForData } from 'components/common'; import { useTabState } from 'components/hooks/useTabState'; +import { secondaryBackgroundColor } from 'components/Theme'; import { Execution } from 'models'; import * as React from 'react'; import { NodeExecutionsRequestConfigContext } from '../contexts'; @@ -23,6 +24,7 @@ const useStyles = makeStyles((theme: Theme) => ({ minHeight: 0 }, tabs: { + background: secondaryBackgroundColor, paddingLeft: theme.spacing(3.5) } })); diff --git a/src/components/Executions/ExecutionDetails/constants.ts b/src/components/Executions/ExecutionDetails/constants.ts new file mode 100644 index 000000000..2e0e26fec --- /dev/null +++ b/src/components/Executions/ExecutionDetails/constants.ts @@ -0,0 +1,7 @@ +export enum ExecutionMetadataLabels { + cluster = 'Cluster', + domain = 'Domain', + duration = 'Duration', + time = 'Time', + version = 'Version' +} diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx new file mode 100644 index 000000000..f1c247318 --- /dev/null +++ b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx @@ -0,0 +1,58 @@ +import { render } from '@testing-library/react'; +import { unknownValueString } from 'common/constants'; +import { Execution } from 'models'; +import { createMockExecution } from 'models/__mocks__/executionsData'; +import * as React from 'react'; +import { ExecutionMetadataLabels } from '../constants'; +import { ExecutionMetadata } from '../ExecutionMetadata'; + +const clusterTestId = `metadata-${ExecutionMetadataLabels.cluster}`; + +describe('ExecutionMetadata', () => { + let execution: Execution; + beforeEach(() => { + execution = createMockExecution(); + }); + + const renderMetadata = () => + render(); + + it('shows cluster name if available', () => { + const { getByTestId } = renderMetadata(); + + expect( + execution.spec.metadata.systemMetadata?.executionCluster + ).toBeDefined(); + expect(getByTestId(clusterTestId)).toHaveTextContent( + execution.spec.metadata.systemMetadata!.executionCluster! + ); + }); + + it('shows unknown string for cluster if no metadata', () => { + delete execution.spec.metadata.systemMetadata; + const { getByTestId } = renderMetadata(); + expect(getByTestId(clusterTestId)).toHaveTextContent( + unknownValueString + ); + }); + + it('shows unknown string for cluster if no cluster name', () => { + delete execution.spec.metadata.systemMetadata?.executionCluster; + const { getByTestId } = renderMetadata(); + expect(getByTestId(clusterTestId)).toHaveTextContent( + unknownValueString + ); + }); + + it('does not show start time if not available', () => { + delete execution.closure.startedAt; + const { queryByText } = renderMetadata(); + expect(queryByText(ExecutionMetadataLabels.time)).toBeNull; + }); + + it('does not show duration if not available', () => { + delete execution.closure.duration; + const { queryByText } = renderMetadata(); + expect(queryByText(ExecutionMetadataLabels.duration)).toBeNull; + }); +}); diff --git a/src/components/Theme/constants.ts b/src/components/Theme/constants.ts index 0af47716c..67861b961 100644 --- a/src/components/Theme/constants.ts +++ b/src/components/Theme/constants.ts @@ -10,6 +10,7 @@ export const primaryColor = COLOR_SPECTRUM.purple60.color; export const primaryLightColor = COLOR_SPECTRUM.purple30.color; export const primaryDarkColor = COLOR_SPECTRUM.purple70.color; export const secondaryColor = COLOR_SPECTRUM.indigo100.color; +export const secondaryBackgroundColor = COLOR_SPECTRUM.gray5.color; export const primaryTextColor = COLOR_SPECTRUM.gray100.color; export const secondaryTextColor = COLOR_SPECTRUM.gray60.color; diff --git a/src/models/__mocks__/executionsData.ts b/src/models/__mocks__/executionsData.ts index b33086bea..a1038728e 100644 --- a/src/models/__mocks__/executionsData.ts +++ b/src/models/__mocks__/executionsData.ts @@ -72,7 +72,10 @@ export function generateExecutionMetadata(): ExecutionMetadata { return { mode: ExecutionMode.MANUAL, nesting: 0, - principal: 'human' + principal: 'human', + systemMetadata: { + executionCluster: 'flyte' + } }; } diff --git a/yarn.lock b/yarn.lock index 62f8c2ed4..e32ded7d0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1780,9 +1780,9 @@ core-js "^2.5.7" "@lyft/flyteidl@^0.17.27": - version "0.17.27" - resolved "https://registry.yarnpkg.com/@lyft/flyteidl/-/flyteidl-0.17.27.tgz#5d7a13f27e1ae6bc775041bb60751abc23883ffa" - integrity sha512-WYyGT9aEfT9Kov75xmFu93kI6mox263ovO9pipWjPecn7hrvtYPfjxcefWsCS8FFfh4JBS7tuv76ELYecZfdOQ== + version "0.17.34" + resolved "https://registry.yarnpkg.com/@lyft/flyteidl/-/flyteidl-0.17.34.tgz#f61ad0bb0824d1505745f656029cb14c9a76e29d" + integrity sha512-g0xpgT8Gzrdr7wY7jvjl7mb3DVFYUyrqePz+4cAp6T0LjUu4WGDnCiFdSzoNP1C3E+vcpHl0SS6L6Bh+bFE0Iw== "@marionebl/sander@^0.6.0": version "0.6.1" @@ -6646,7 +6646,7 @@ debug@^3.0.0, debug@^3.1.0, debug@^3.2.5: dependencies: ms "^2.1.1" -debuglog@*, debuglog@^1.0.1: +debuglog@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/debuglog/-/debuglog-1.0.1.tgz#aa24ffb9ac3df9a2351837cfb2d279360cd78492" integrity sha1-qiT/uaw9+aI1GDfPstJ5NgzXhJI= @@ -9163,7 +9163,7 @@ import-local@^3.0.2: pkg-dir "^4.2.0" resolve-cwd "^3.0.0" -imurmurhash@*, imurmurhash@^0.1.4: +imurmurhash@^0.1.4: version "0.1.4" resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea" integrity sha1-khi5srkoojixPcT7a21XbyMUU+o= @@ -11002,11 +11002,6 @@ lodash._basecopy@^3.0.0: resolved "https://registry.yarnpkg.com/lodash._basecopy/-/lodash._basecopy-3.0.1.tgz#8da0e6a876cf344c0ad8a54882111dd3c5c7ca36" integrity sha1-jaDmqHbPNEwK2KVIghEd08XHyjY= -lodash._baseindexof@*: - version "3.1.0" - resolved "https://registry.yarnpkg.com/lodash._baseindexof/-/lodash._baseindexof-3.1.0.tgz#fe52b53a1c6761e42618d654e4a25789ed61822c" - integrity sha1-/lK1OhxnYeQmGNZU5KJXie1hgiw= - lodash._baseuniq@~4.6.0: version "4.6.0" resolved "https://registry.yarnpkg.com/lodash._baseuniq/-/lodash._baseuniq-4.6.0.tgz#0ebb44e456814af7905c6212fa2c9b2d51b841e8" @@ -11015,16 +11010,11 @@ lodash._baseuniq@~4.6.0: lodash._createset "~4.0.0" lodash._root "~3.0.0" -lodash._bindcallback@*, lodash._bindcallback@^3.0.0: +lodash._bindcallback@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/lodash._bindcallback/-/lodash._bindcallback-3.0.1.tgz#e531c27644cf8b57a99e17ed95b35c748789392e" integrity sha1-5THCdkTPi1epnhftlbNcdIeJOS4= -lodash._cacheindexof@*: - version "3.0.2" - resolved "https://registry.yarnpkg.com/lodash._cacheindexof/-/lodash._cacheindexof-3.0.2.tgz#3dc69ac82498d2ee5e3ce56091bafd2adc7bde92" - integrity sha1-PcaayCSY0u5ePOVgkbr9Ktx73pI= - lodash._createassigner@^3.0.0: version "3.1.1" resolved "https://registry.yarnpkg.com/lodash._createassigner/-/lodash._createassigner-3.1.1.tgz#838a5bae2fdaca63ac22dee8e19fa4e6d6970b11" @@ -11034,19 +11024,12 @@ lodash._createassigner@^3.0.0: lodash._isiterateecall "^3.0.0" lodash.restparam "^3.0.0" -lodash._createcache@*: - version "3.1.2" - resolved "https://registry.yarnpkg.com/lodash._createcache/-/lodash._createcache-3.1.2.tgz#56d6a064017625e79ebca6b8018e17440bdcf093" - integrity sha1-VtagZAF2JeeevKa4AY4XRAvc8JM= - dependencies: - lodash._getnative "^3.0.0" - lodash._createset@~4.0.0: version "4.0.3" resolved "https://registry.yarnpkg.com/lodash._createset/-/lodash._createset-4.0.3.tgz#0f4659fbb09d75194fa9e2b88a6644d363c9fe26" integrity sha1-D0ZZ+7CddRlPqeK4imZE02PJ/iY= -lodash._getnative@*, lodash._getnative@^3.0.0: +lodash._getnative@^3.0.0: version "3.9.1" resolved "https://registry.yarnpkg.com/lodash._getnative/-/lodash._getnative-3.9.1.tgz#570bc7dede46d61cdcde687d65d3eecbaa3aaff5" integrity sha1-VwvH3t5G1hzc3mh9ZdPuy6o6r/U= @@ -11167,7 +11150,7 @@ lodash.memoize@4.x, lodash.memoize@^4.1.2: resolved "https://registry.yarnpkg.com/lodash.memoize/-/lodash.memoize-4.1.2.tgz#bcc6c49a42a2840ed997f323eada5ecd182e0bfe" integrity sha1-vMbEmkKihA7Zl/Mj6tpezRguC/4= -lodash.restparam@*, lodash.restparam@^3.0.0: +lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU=