From 7a595161c181b4a21ef62bfcc5c70adf3feec7b7 Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Thu, 9 May 2024 04:11:22 -0400 Subject: [PATCH] [RHOAIENG-4825] Use execution input/output values as fallback for run node details --- .../pipelineRun/PipelineRunDetails.tsx | 1 + .../pipelineRun/PipelineRunDetailsActions.tsx | 2 +- .../PipelineRunDrawerRightContent.tsx | 5 +- .../PipelineRunDrawerRightTabs.tsx | 14 +- .../SelectedNodeInputOutputTab.tsx | 90 ++++++++++++- .../SelectedNodeInputOutputTab.spec.tsx | 122 ++++++++++++++++++ .../taskDetails/TaskDetailsPrintKeyValues.tsx | 2 +- .../ArtifactPropertyDescriptionList.tsx | 6 +- .../ExecutionDetailsPropertiesValue.tsx | 28 +++- 9 files changed, 250 insertions(+), 20 deletions(-) create mode 100644 frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/__tests__/SelectedNodeInputOutputTab.spec.tsx diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx index f0ce22a4cb..231d291403 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetails.tsx @@ -96,6 +96,7 @@ const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath, setSelectedId(null)} + executions={executions} /> } > diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsActions.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsActions.tsx index 700d7c9ab9..d7fac1de30 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsActions.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDetailsActions.tsx @@ -76,7 +76,7 @@ const PipelineRunDetailsActions: React.FC = ({ o Compare runs ) : ( - <> + ), , onDelete()}> diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx index 6240bb6b87..398af65de5 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightContent.tsx @@ -11,14 +11,17 @@ import { import PipelineRunDrawerRightTabs from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs'; import './PipelineRunDrawer.scss'; import { PipelineTask } from '~/concepts/pipelines/topology'; +import { Execution } from '~/third_party/mlmd'; type PipelineRunDrawerRightContentProps = { task?: PipelineTask; + executions: Execution[]; onClose: () => void; }; const PipelineRunDrawerRightContent: React.FC = ({ task, + executions, onClose, }) => { if (!task) { @@ -42,7 +45,7 @@ const PipelineRunDrawerRightContent: React.FC - + ); diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx index 655ce72af6..b75f039eb4 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunDrawerRightTabs.tsx @@ -6,6 +6,7 @@ import LogsTab from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/r import './PipelineRunDrawer.scss'; import { PipelineTask } from '~/concepts/pipelines/topology'; import SelectedNodeVolumeMountsTab from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/SelectedNodeVolumeMountsTab'; +import { Execution } from '~/third_party/mlmd'; enum PipelineRunNodeTabs { INPUT_OUTPUT = 'inputoutput', @@ -27,13 +28,22 @@ const PipelineRunNodeTabsTitles = { type PipelineRunDrawerRightTabsProps = { task: PipelineTask; + executions: Execution[]; }; -const PipelineRunDrawerRightTabs: React.FC = ({ task }) => { +const PipelineRunDrawerRightTabs: React.FC = ({ + task, + executions, +}) => { const [selection, setSelection] = React.useState(PipelineRunNodeTabs.INPUT_OUTPUT); + const taskExecution = executions.find( + (e) => e.getCustomPropertiesMap().get('task_name')?.getStringValue() === task.name, + ); const tabContents: Record = { - [PipelineRunNodeTabs.INPUT_OUTPUT]: , + [PipelineRunNodeTabs.INPUT_OUTPUT]: ( + + ), // [PipelineRunNodeTabs.VISUALIZATIONS]: <>TBD 2, [PipelineRunNodeTabs.DETAILS]: , [PipelineRunNodeTabs.VOLUMES]: , diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/SelectedNodeInputOutputTab.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/SelectedNodeInputOutputTab.tsx index 76f755e789..71b4b99d73 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/SelectedNodeInputOutputTab.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/SelectedNodeInputOutputTab.tsx @@ -1,13 +1,93 @@ -import * as React from 'react'; +import React from 'react'; + import { Stack, StackItem } from '@patternfly/react-core'; +import { Value as ProtoValue } from 'google-protobuf/google/protobuf/struct_pb'; + +import { Execution } from '~/third_party/mlmd'; import TaskDetailsInputOutput from '~/concepts/pipelines/content/pipelinesDetails/taskDetails/TaskDetailsInputOutput'; -import { PipelineTask } from '~/concepts/pipelines/topology'; +import { PipelineTask, PipelineTaskParam } from '~/concepts/pipelines/topology'; +import { ExecutionDetailsPropertiesValueCode } from '~/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue'; +import { InputDefinitionParameterType } from '~/concepts/pipelines/kfTypes'; +import { NoValue } from '~/components/NoValue'; type SelectedNodeInputOutputTabProps = { task: PipelineTask; + execution: Execution.AsObject | undefined; }; -const SelectedNodeInputOutputTab: React.FC = ({ task }) => { +const SelectedNodeInputOutputTab: React.FC = ({ + task, + execution, +}) => { + const getExecutionFieldsMap = React.useCallback( + (key: string) => + execution?.customPropertiesMap.find(([customPropKey]) => customPropKey === key)?.[1] + .structValue?.fieldsMap || [], + [execution?.customPropertiesMap], + ); + + const getExecutionValueFromInputType = React.useCallback( + (executionValues: ProtoValue.AsObject | undefined, type: InputDefinitionParameterType) => { + const { numberValue, boolValue, stringValue, listValue, structValue, nullValue } = + executionValues || {}; + + switch (type) { + case InputDefinitionParameterType.DOUBLE: + case InputDefinitionParameterType.INTEGER: + return numberValue; + case InputDefinitionParameterType.BOOLEAN: + return boolValue ? 'True' : 'False'; + case InputDefinitionParameterType.STRING: + try { + const jsonStringValue = JSON.parse(stringValue ?? ''); + + if (parseFloat(jsonStringValue)) { + throw stringValue; + } + + return ( + + ); + } catch { + return stringValue || ; + } + break; + case InputDefinitionParameterType.LIST: + return ; + case InputDefinitionParameterType.STRUCT: + return ( + + ); + default: + return nullValue; + } + }, + [], + ); + + const getParams = React.useCallback( + ( + params: PipelineTaskParam[] | undefined, + executionParamMapList: [string, ProtoValue.AsObject][], + ) => + params?.map((taskParam) => { + const { label, value, type } = taskParam; + const executionParamMap = executionParamMapList.find( + ([executionInputKey]) => taskParam.label === executionInputKey, + ); + const { 1: executionParamValues } = executionParamMap || []; + const executionValue = getExecutionValueFromInputType(executionParamValues, type); + + return { + label, + value: executionValue || value || type, + }; + }), + [getExecutionValueFromInputType], + ); + if (!task.inputs && !task.outputs) { return <>No content; } @@ -19,7 +99,7 @@ const SelectedNodeInputOutputTab: React.FC = ({ ({ label: a.label, value: a.type }))} - params={task.inputs.params?.map((p) => ({ label: p.label, value: p.value ?? p.type }))} + params={getParams(task.inputs.params, getExecutionFieldsMap('inputs'))} /> )} @@ -28,7 +108,7 @@ const SelectedNodeInputOutputTab: React.FC = ({ ({ label: a.label, value: a.type }))} - params={task.outputs.params?.map((p) => ({ label: p.label, value: p.value ?? p.type }))} + params={getParams(task.outputs.params, getExecutionFieldsMap('outputs'))} /> )} diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/__tests__/SelectedNodeInputOutputTab.spec.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/__tests__/SelectedNodeInputOutputTab.spec.tsx new file mode 100644 index 0000000000..0357f3cbf1 --- /dev/null +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/__tests__/SelectedNodeInputOutputTab.spec.tsx @@ -0,0 +1,122 @@ +import React from 'react'; + +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +import SelectedNodeInputOutputTab from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/SelectedNodeInputOutputTab'; +import { InputDefinitionParameterType } from '~/concepts/pipelines/kfTypes'; +import { Execution } from '~/third_party/mlmd'; + +jest.mock('~/components/MaxHeightCodeEditor', () => ({ + MaxHeightCodeEditor: ({ code }: { code: string }) => JSON.stringify(JSON.parse(code)), +})); + +describe('SelectedNodeInputOutputTab', () => { + it('renders execution values for input parameters', async () => { + render( + , + ); + + expect(screen.getByText('Some string')).toBeVisible(); + expect(screen.getByText('No value')).toBeVisible(); + expect(screen.getByText('{"some":"stringObject"}')).toBeVisible(); + expect(screen.getByText('1')).toBeVisible(); + expect(screen.getByText('12.15')).toBeVisible(); + expect(screen.getByText('True')).toBeVisible(); + expect(screen.getByText('{"some":"object"}')).toBeVisible(); + expect(screen.getByText('[{"list":"item-1"},{"list":"item-2"}]')).toBeVisible(); + }); + + it('renders execution values for output parameters', async () => { + render( + , + ); + + expect(screen.getByText('Some string')).toBeVisible(); + expect(screen.getByText('No value')).toBeVisible(); + expect(screen.getByText('{"some":"stringObject"}')).toBeVisible(); + expect(screen.getByText('1')).toBeVisible(); + expect(screen.getByText('12.15')).toBeVisible(); + expect(screen.getByText('True')).toBeVisible(); + expect(screen.getByText('{"some":"object"}')).toBeVisible(); + expect(screen.getByText('[{"list":"item-1"},{"list":"item-2"}]')).toBeVisible(); + }); +}); + +const taskParams = [ + { label: 'param-string-1', type: InputDefinitionParameterType.STRING }, + { label: 'param-string-2', type: InputDefinitionParameterType.STRING }, + { label: 'param-string-3', type: InputDefinitionParameterType.STRING }, + { label: 'param-string-4', type: InputDefinitionParameterType.STRING }, + { label: 'param-int', type: InputDefinitionParameterType.INTEGER }, + { label: 'param-double', type: InputDefinitionParameterType.DOUBLE }, + { label: 'param-bool', type: InputDefinitionParameterType.BOOLEAN }, + { label: 'param-struct', type: InputDefinitionParameterType.STRUCT }, + { label: 'param-list', type: InputDefinitionParameterType.LIST }, +]; + +const executionFieldsMap = [ + ['param-string-1', { stringValue: 'Some string' }], + ['param-string-2', { stringValue: '' }], + ['param-string-3', { stringValue: '10.10' }], + ['param-string-4', { stringValue: '{"some":"stringObject"}' }], + ['param-int', { numberValue: 1 }], + ['param-double', { numberValue: 12.15 }], + ['param-bool', { boolValue: true }], + [ + 'param-struct', + { + structValue: { some: 'object' }, + }, + ], + [ + 'param-list', + { + listValue: [{ list: 'item-1' }, { list: 'item-2' }], + }, + ], +]; diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/taskDetails/TaskDetailsPrintKeyValues.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/taskDetails/TaskDetailsPrintKeyValues.tsx index 33b7be52ac..5ecd331b4b 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/taskDetails/TaskDetailsPrintKeyValues.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/taskDetails/TaskDetailsPrintKeyValues.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { Grid, GridItem, Truncate } from '@patternfly/react-core'; type TaskDetailsPrintKeyValuesProps = { - items: { label: string; value: string }[]; + items: { label: string; value: React.ReactNode }[]; }; const TaskDetailsPrintKeyValues: React.FC = ({ items }) => ( diff --git a/frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactDetails/ArtifactPropertyDescriptionList.tsx b/frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactDetails/ArtifactPropertyDescriptionList.tsx index 2412e5883f..b0e16076d7 100644 --- a/frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactDetails/ArtifactPropertyDescriptionList.tsx +++ b/frontend/src/pages/pipelines/global/experiments/artifacts/ArtifactDetails/ArtifactPropertyDescriptionList.tsx @@ -9,7 +9,7 @@ import { import { Value } from '~/third_party/mlmd'; import { NoValue } from '~/components/NoValue'; -import { MaxHeightCodeEditor } from '~/components/MaxHeightCodeEditor'; +import { ExecutionDetailsPropertiesValueCode } from '~/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue'; interface ArtifactPropertyDescriptionListProps { testId?: string; @@ -26,9 +26,7 @@ export const ArtifactPropertyDescriptionList: React.FC ); diff --git a/frontend/src/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue.tsx b/frontend/src/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue.tsx index 3669b61a9c..ecf04a6130 100644 --- a/frontend/src/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue.tsx +++ b/frontend/src/pages/pipelines/global/experiments/executions/details/ExecutionDetailsPropertiesValue.tsx @@ -1,13 +1,21 @@ import React from 'react'; import { MlmdMetadataValueType } from '~/pages/pipelines/global/experiments/executions/utils'; import { MaxHeightCodeEditor } from '~/components/MaxHeightCodeEditor'; +import { NoValue } from '~/components/NoValue'; type ExecutionDetailsPropertiesValueProps = { value: MlmdMetadataValueType; }; -const ExecutionDetailsPropertiesValueCode = ({ code }: { code: string }) => ( - +export const ExecutionDetailsPropertiesValueCode: React.FC< + Pick, 'code'> +> = ({ code }) => ( + ); const ExecutionDetailsPropertiesValue: React.FC = ({ @@ -16,18 +24,26 @@ const ExecutionDetailsPropertiesValue: React.FC; } catch { // not JSON, return directly - return value; + return value || ; } } - if (typeof value === 'number') { - return value; - } + // value is Struct const jsObject = value.toJavaScript(); // When Struct is converted to js object, it may contain a top level "struct"