Skip to content

Commit

Permalink
Merge pull request #2787 from jpuzz0/RHOAIENG-4825-input-output-mlmd-…
Browse files Browse the repository at this point in the history
…params

[RHOAIENG-4825] Use execution input/output values as fallback for run node details
  • Loading branch information
openshift-merge-bot[bot] authored May 9, 2024
2 parents fa06535 + 7a59516 commit ee7e684
Show file tree
Hide file tree
Showing 9 changed files with 250 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath,
<PipelineRunDrawerRightContent
task={selectedNode?.data.pipelineTask}
onClose={() => setSelectedId(null)}
executions={executions}
/>
}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const PipelineRunDetailsActions: React.FC<PipelineRunDetailsActionsProps> = ({ o
Compare runs
</DropdownItem>
) : (
<></>
<React.Fragment key="compare-runs" />
),
<DropdownSeparator key="separator" />,
<DropdownItem key="delete-run" onClick={() => onDelete()}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<PipelineRunDrawerRightContentProps> = ({
task,
executions,
onClose,
}) => {
if (!task) {
Expand All @@ -42,7 +45,7 @@ const PipelineRunDrawerRightContent: React.FC<PipelineRunDrawerRightContentProps
</DrawerActions>
</DrawerHead>
<DrawerPanelBody className="pipeline-run__drawer-panel-body pf-v5-u-pr-sm">
<PipelineRunDrawerRightTabs task={task} />
<PipelineRunDrawerRightTabs task={task} executions={executions} />
</DrawerPanelBody>
</DrawerPanelContent>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -27,13 +28,22 @@ const PipelineRunNodeTabsTitles = {

type PipelineRunDrawerRightTabsProps = {
task: PipelineTask;
executions: Execution[];
};

const PipelineRunDrawerRightTabs: React.FC<PipelineRunDrawerRightTabsProps> = ({ task }) => {
const PipelineRunDrawerRightTabs: React.FC<PipelineRunDrawerRightTabsProps> = ({
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, React.ReactNode> = {
[PipelineRunNodeTabs.INPUT_OUTPUT]: <SelectedNodeInputOutputTab task={task} />,
[PipelineRunNodeTabs.INPUT_OUTPUT]: (
<SelectedNodeInputOutputTab task={task} execution={taskExecution?.toObject()} />
),
// [PipelineRunNodeTabs.VISUALIZATIONS]: <>TBD 2</>,
[PipelineRunNodeTabs.DETAILS]: <SelectedNodeDetailsTab task={task} />,
[PipelineRunNodeTabs.VOLUMES]: <SelectedNodeVolumeMountsTab task={task} />,
Expand Down
Original file line number Diff line number Diff line change
@@ -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<SelectedNodeInputOutputTabProps> = ({ task }) => {
const SelectedNodeInputOutputTab: React.FC<SelectedNodeInputOutputTabProps> = ({
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 (
<ExecutionDetailsPropertiesValueCode
code={JSON.stringify(jsonStringValue, null, 2)}
/>
);
} catch {
return stringValue || <NoValue />;
}
break;
case InputDefinitionParameterType.LIST:
return <ExecutionDetailsPropertiesValueCode code={JSON.stringify(listValue, null, 2)} />;
case InputDefinitionParameterType.STRUCT:
return (
<ExecutionDetailsPropertiesValueCode code={JSON.stringify(structValue, null, 2)} />
);
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</>;
}
Expand All @@ -19,7 +99,7 @@ const SelectedNodeInputOutputTab: React.FC<SelectedNodeInputOutputTabProps> = ({
<TaskDetailsInputOutput
type="Input"
artifacts={task.inputs.artifacts?.map((a) => ({ 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'))}
/>
</StackItem>
)}
Expand All @@ -28,7 +108,7 @@ const SelectedNodeInputOutputTab: React.FC<SelectedNodeInputOutputTabProps> = ({
<TaskDetailsInputOutput
type="Output"
artifacts={task.outputs.artifacts?.map((a) => ({ 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'))}
/>
</StackItem>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -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(
<SelectedNodeInputOutputTab
task={{
type: 'groupTask',
name: 'task-1',
inputs: {
params: taskParams,
},
}}
execution={
{
customPropertiesMap: [
[
'inputs',
{
structValue: {
fieldsMap: executionFieldsMap,
},
},
],
],
} as unknown as Execution.AsObject
}
/>,
);

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(
<SelectedNodeInputOutputTab
task={{
type: 'groupTask',
name: 'task-1',
outputs: {
params: taskParams,
},
}}
execution={
{
customPropertiesMap: [
[
'outputs',
{
structValue: {
fieldsMap: executionFieldsMap,
},
},
],
],
} as unknown as Execution.AsObject
}
/>,
);

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' }],
},
],
];
Original file line number Diff line number Diff line change
Expand Up @@ -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<TaskDetailsPrintKeyValuesProps> = ({ items }) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,9 +26,7 @@ export const ArtifactPropertyDescriptionList: React.FC<ArtifactPropertyDescripti

if (property.structValue || property.protoValue) {
propValue = (
<MaxHeightCodeEditor
isReadOnly
maxHeight={300}
<ExecutionDetailsPropertiesValueCode
code={JSON.stringify(property.structValue || property.protoValue, null, 2)}
/>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -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 }) => (
<MaxHeightCodeEditor isReadOnly code={code} maxHeight={300} />
export const ExecutionDetailsPropertiesValueCode: React.FC<
Pick<React.ComponentProps<typeof MaxHeightCodeEditor>, 'code'>
> = ({ code }) => (
<MaxHeightCodeEditor
isReadOnly
code={code}
maxHeight={300}
data-testid="execution-value-code-editor"
/>
);

const ExecutionDetailsPropertiesValue: React.FC<ExecutionDetailsPropertiesValueProps> = ({
Expand All @@ -16,18 +24,26 @@ const ExecutionDetailsPropertiesValue: React.FC<ExecutionDetailsPropertiesValueP
if (!value) {
return '';
}

if (typeof value === 'number' || (typeof value === 'string' && !Number.isNaN(value))) {
return value;
}

if (typeof value === 'string') {
try {
const jsonValue = JSON.parse(value);

if (parseFloat(jsonValue ?? '')) {
throw value;
}

return <ExecutionDetailsPropertiesValueCode code={JSON.stringify(jsonValue, null, 2)} />;
} catch {
// not JSON, return directly
return value;
return value || <NoValue />;
}
}
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"
Expand Down

0 comments on commit ee7e684

Please sign in to comment.