From 89da852bc4503da3fd5be833e2e66bf238c8caf0 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 30 Jun 2020 12:57:35 -0700 Subject: [PATCH 1/8] chore: update flyteidl --- package.json | 2 +- yarn.lock | 33 ++++++++------------------------- 2 files changed, 9 insertions(+), 26 deletions(-) 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/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= From 59a6f172a6e52791c12eefaee4e237249f664bd5 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 30 Jun 2020 15:37:25 -0700 Subject: [PATCH 2/8] fix: move some of execution content out of page header --- .../ExecutionDetails/ExecutionDetails.tsx | 2 + .../ExecutionDetailsAppBarContent.tsx | 99 ++++--------------- .../ExecutionDetails/ExecutionMetadata.tsx | 88 +++++++++++++++++ .../ExecutionDetails/ExecutionNodeViews.tsx | 2 + src/components/Theme/constants.ts | 1 + 5 files changed, 113 insertions(+), 79 deletions(-) create mode 100644 src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx index ecf511921..81bd33929 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx @@ -8,6 +8,7 @@ import { useExecutionDataCache } from '../useExecutionDataCache'; import { useWorkflowExecution } from '../useWorkflowExecution'; import { executionIsTerminal } from '../utils'; import { ExecutionDetailsAppBarContent } from './ExecutionDetailsAppBarContent'; +import { ExecutionMetadata } from './ExecutionMetadata'; import { ExecutionNodeViews } from './ExecutionNodeViews'; export interface ExecutionDetailsRouteParams { @@ -47,6 +48,7 @@ export const ExecutionDetailsContainer: React.FC = + 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: { + alignItems: 'center', + background: secondaryBackgroundColor, + display: 'flex', + flex: '0 1 auto', + paddingTop: theme.spacing(3), + paddingBottom: theme.spacing(2) + }, + detailItem: { + flexShrink: 0, + marginLeft: theme.spacing(4) + }, + 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, startedAt, workflowId } = execution.closure; + + const details: DetailItem[] = [ + { 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 ( +
+ {details.map(({ className, label, value }, idx) => ( +
+ + {label} + + + {value} + +
+ ))} +
+ ); +}; 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/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; From 549239782c195a07242b7b6937b0bacce4a30894 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 30 Jun 2020 15:39:39 -0700 Subject: [PATCH 3/8] fix: show cluster name if available --- .../Executions/ExecutionDetails/ExecutionMetadata.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx index 59659cda4..12c8063f6 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx @@ -1,6 +1,7 @@ import { Typography } from '@material-ui/core'; import { makeStyles, Theme } from '@material-ui/core/styles'; import * as classnames from 'classnames'; +import { unknownValueString } from 'common/constants'; import { formatDateUTC, protobufDurationToHMS } from 'common/formatters'; import { timestampToDate } from 'common/utils'; import { useCommonStyles } from 'components/common/styles'; @@ -44,6 +45,11 @@ export const ExecutionMetadata: React.FC<{ const { domain } = execution.id; const { duration, startedAt, workflowId } = execution.closure; + const { systemMetadata } = execution.spec.metadata; + const cluster = + systemMetadata && systemMetadata.executionCluster + ? systemMetadata.executionCluster + : unknownValueString; const details: DetailItem[] = [ { label: 'Domain', value: domain }, @@ -59,6 +65,10 @@ export const ExecutionMetadata: React.FC<{ { label: 'Duration', value: duration ? protobufDurationToHMS(duration) : '' + }, + { + label: 'Cluster', + value: cluster } ]; From ebeee5a5412034df5ca5850241ba73e8cf0af97f Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Tue, 30 Jun 2020 16:16:31 -0700 Subject: [PATCH 4/8] refactor: rearranging metadata display and adding cluster info --- .../ExecutionDetails/ExecutionMetadata.tsx | 12 +++-- .../Executions/ExecutionDetails/constants.ts | 7 +++ .../test/ExecutionMetadata.test.tsx | 46 +++++++++++++++++++ src/models/__mocks__/executionsData.ts | 5 +- 4 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 src/components/Executions/ExecutionDetails/constants.ts create mode 100644 src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx index 12c8063f6..e9c05848d 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx @@ -8,6 +8,7 @@ import { useCommonStyles } from 'components/common/styles'; import { secondaryBackgroundColor, smallFontSize } from 'components/Theme'; import { Execution } from 'models'; import * as React from 'react'; +import { executionMetadataLabels } from './constants'; const useStyles = makeStyles((theme: Theme) => { return { @@ -52,22 +53,22 @@ export const ExecutionMetadata: React.FC<{ : unknownValueString; const details: DetailItem[] = [ - { label: 'Domain', value: domain }, + { label: executionMetadataLabels.domain, value: domain }, { className: styles.version, - label: 'Version', + label: executionMetadataLabels.version, value: workflowId.version }, { - label: 'Time', + label: executionMetadataLabels.time, value: startedAt ? formatDateUTC(timestampToDate(startedAt)) : '' }, { - label: 'Duration', + label: executionMetadataLabels.duration, value: duration ? protobufDurationToHMS(duration) : '' }, { - label: 'Cluster', + label: executionMetadataLabels.cluster, value: cluster } ]; @@ -88,6 +89,7 @@ export const ExecutionMetadata: React.FC<{ {value} diff --git a/src/components/Executions/ExecutionDetails/constants.ts b/src/components/Executions/ExecutionDetails/constants.ts new file mode 100644 index 000000000..ebb6c3fc7 --- /dev/null +++ b/src/components/Executions/ExecutionDetails/constants.ts @@ -0,0 +1,7 @@ +export const 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..640d36315 --- /dev/null +++ b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx @@ -0,0 +1,46 @@ +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 + ); + }); +}); 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' + } }; } From a96b8058f59fa9ef983fda55e6667f52825954e6 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 1 Jul 2020 11:04:24 -0700 Subject: [PATCH 5/8] refactor: pr feedback --- .../ExecutionDetails/ExecutionMetadata.tsx | 17 +++++++---------- .../Executions/ExecutionDetails/constants.ts | 14 +++++++------- .../test/ExecutionMetadata.test.tsx | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx index e9c05848d..b18445e59 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx @@ -8,7 +8,7 @@ import { useCommonStyles } from 'components/common/styles'; import { secondaryBackgroundColor, smallFontSize } from 'components/Theme'; import { Execution } from 'models'; import * as React from 'react'; -import { executionMetadataLabels } from './constants'; +import { ExecutionMetadataLabels } from './constants'; const useStyles = makeStyles((theme: Theme) => { return { @@ -47,28 +47,25 @@ export const ExecutionMetadata: React.FC<{ const { domain } = execution.id; const { duration, startedAt, workflowId } = execution.closure; const { systemMetadata } = execution.spec.metadata; - const cluster = - systemMetadata && systemMetadata.executionCluster - ? systemMetadata.executionCluster - : unknownValueString; + const cluster = systemMetadata?.executionCluster ?? unknownValueString; const details: DetailItem[] = [ - { label: executionMetadataLabels.domain, value: domain }, + { label: ExecutionMetadataLabels.domain, value: domain }, { className: styles.version, - label: executionMetadataLabels.version, + label: ExecutionMetadataLabels.version, value: workflowId.version }, { - label: executionMetadataLabels.time, + label: ExecutionMetadataLabels.time, value: startedAt ? formatDateUTC(timestampToDate(startedAt)) : '' }, { - label: executionMetadataLabels.duration, + label: ExecutionMetadataLabels.duration, value: duration ? protobufDurationToHMS(duration) : '' }, { - label: executionMetadataLabels.cluster, + label: ExecutionMetadataLabels.cluster, value: cluster } ]; diff --git a/src/components/Executions/ExecutionDetails/constants.ts b/src/components/Executions/ExecutionDetails/constants.ts index ebb6c3fc7..2e0e26fec 100644 --- a/src/components/Executions/ExecutionDetails/constants.ts +++ b/src/components/Executions/ExecutionDetails/constants.ts @@ -1,7 +1,7 @@ -export const executionMetadataLabels = { - cluster: 'Cluster', - domain: 'Domain', - duration: 'Duration', - time: 'Time', - version: 'Version' -}; +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 index 640d36315..6798f2482 100644 --- a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx +++ b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx @@ -3,10 +3,10 @@ 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 { ExecutionMetadataLabels } from '../constants'; import { ExecutionMetadata } from '../ExecutionMetadata'; -const clusterTestId = `metadata-${executionMetadataLabels.cluster}`; +const clusterTestId = `metadata-${ExecutionMetadataLabels.cluster}`; describe('ExecutionMetadata', () => { let execution: Execution; From ff5d7b4d50f291cdccc75f1617abec36ec86382a Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 1 Jul 2020 11:17:00 -0700 Subject: [PATCH 6/8] fix: also show execution errors when available --- .../ExecutionDetails/ExecutionMetadata.tsx | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx index b18445e59..4e09bb3d9 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx @@ -5,17 +5,22 @@ import { unknownValueString } from 'common/constants'; import { formatDateUTC, protobufDurationToHMS } from 'common/formatters'; import { timestampToDate } from 'common/utils'; import { useCommonStyles } from 'components/common/styles'; -import { secondaryBackgroundColor, smallFontSize } from 'components/Theme'; +import { secondaryBackgroundColor } from 'components/Theme'; import { Execution } from 'models'; import * as React from 'react'; +import { ExpandableExecutionError } from '../Tables/ExpandableExecutionError'; import { ExecutionMetadataLabels } from './constants'; const useStyles = makeStyles((theme: Theme) => { return { container: { - alignItems: 'center', background: secondaryBackgroundColor, display: 'flex', + flexDirection: 'column' + }, + detailsContainer: { + alignItems: 'center', + display: 'flex', flex: '0 1 auto', paddingTop: theme.spacing(3), paddingBottom: theme.spacing(2) @@ -45,7 +50,7 @@ export const ExecutionMetadata: React.FC<{ const styles = useStyles(); const { domain } = execution.id; - const { duration, startedAt, workflowId } = execution.closure; + const { duration, error, startedAt, workflowId } = execution.closure; const { systemMetadata } = execution.spec.metadata; const cluster = systemMetadata?.executionCluster ?? unknownValueString; @@ -72,26 +77,30 @@ export const ExecutionMetadata: React.FC<{ return (
- {details.map(({ className, label, value }, idx) => ( -
- - {label} - - + {details.map(({ className, label, value }, idx) => ( +
- {value} - -
- ))} + + {label} + + + {value} + +
+ ))} +
+ + {error ? : null}
); }; From 938be580afc769aa857e52bdc2588212c0c0fe8e Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 1 Jul 2020 12:34:54 -0700 Subject: [PATCH 7/8] fix: makes execution metadata collapsible --- .../ExecutionDetails/ExecutionDetails.tsx | 49 ++++++++++++++++++- .../ExecutionDetails/ExecutionMetadata.tsx | 16 +++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/ExecutionDetails.tsx index 81bd33929..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'; @@ -11,6 +15,29 @@ 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; @@ -34,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, @@ -44,11 +74,28 @@ export const ExecutionDetailsContainer: React.FC = terminateExecution, execution: fetchable.value }; + return ( - +
+ + + +
+ + + +
+
diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx index 4e09bb3d9..d862e3758 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx @@ -16,7 +16,8 @@ const useStyles = makeStyles((theme: Theme) => { container: { background: secondaryBackgroundColor, display: 'flex', - flexDirection: 'column' + flexDirection: 'column', + position: 'relative' }, detailsContainer: { alignItems: 'center', @@ -29,6 +30,19 @@ const useStyles = makeStyles((theme: Theme) => { 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' From 2c012bbf76416b8cd3dd3b2b02b9132f6067663b Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Wed, 1 Jul 2020 12:47:50 -0700 Subject: [PATCH 8/8] fix: don't show execution metadata without values --- .../ExecutionDetails/ExecutionMetadata.tsx | 20 +++++++++++-------- .../test/ExecutionMetadata.test.tsx | 12 +++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx index d862e3758..e6ec500e9 100644 --- a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx +++ b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx @@ -75,19 +75,23 @@ export const ExecutionMetadata: React.FC<{ label: ExecutionMetadataLabels.version, value: workflowId.version }, - { - label: ExecutionMetadataLabels.time, - value: startedAt ? formatDateUTC(timestampToDate(startedAt)) : '' - }, - { - label: ExecutionMetadataLabels.duration, - value: duration ? protobufDurationToHMS(duration) : '' - }, { 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 (
diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx index 6798f2482..f1c247318 100644 --- a/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx +++ b/src/components/Executions/ExecutionDetails/test/ExecutionMetadata.test.tsx @@ -43,4 +43,16 @@ describe('ExecutionMetadata', () => { 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; + }); });