From 049be11653f6879e40a00fb7037715c88bfd0e36 Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Fri, 7 Jun 2024 15:27:31 -0400 Subject: [PATCH] [RHOAIENG-7481] Add metrics columns to pipeline run table and modal param selector --- .../cypress/pages/pipelines/compareRuns.ts | 2 +- frontend/src/components/table/CheckboxTd.tsx | 8 +- frontend/src/components/table/TableBase.tsx | 33 ++-- frontend/src/components/table/types.ts | 9 +- .../__tests__/useGetArtifactsByRuns.spec.ts | 155 +++++++++++++++ .../apiHooks/mlmd/useGetArtifactsByRuns.ts | 43 +++++ .../artifacts/ArtifactVisualization.tsx | 22 +-- .../artifacts/__tests__/utils.spec.ts | 31 +++ .../pipelineRun/artifacts/utils.ts | 26 +++ .../pipelines/content/tables/columns.ts | 47 +++++ .../pipelineRun/CustomMetricsColumnsModal.tsx | 181 ++++++++++++++++++ .../tables/pipelineRun/PipelineRunTable.tsx | 132 +++++++++++-- .../pipelineRun/PipelineRunTableRow.tsx | 14 +- .../content/tables/pipelineRun/utils.ts | 3 + 14 files changed, 658 insertions(+), 48 deletions(-) create mode 100644 frontend/src/concepts/pipelines/apiHooks/mlmd/__tests__/useGetArtifactsByRuns.spec.ts create mode 100644 frontend/src/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns.ts create mode 100644 frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/__tests__/utils.spec.ts create mode 100644 frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/utils.ts create mode 100644 frontend/src/concepts/pipelines/content/tables/pipelineRun/CustomMetricsColumnsModal.tsx create mode 100644 frontend/src/concepts/pipelines/content/tables/pipelineRun/utils.ts diff --git a/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts b/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts index e1c23b9529..c4fb2dcf0a 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts @@ -44,7 +44,7 @@ class CompareRunParamsTable { } findEmptyState() { - return this.find().parent().findByTestId('compare-runs-params-empty-state'); + return this.find().parent().parent().findByTestId('compare-runs-params-empty-state'); } findColumnByName(name: string) { diff --git a/frontend/src/components/table/CheckboxTd.tsx b/frontend/src/components/table/CheckboxTd.tsx index 3d689d1381..b6393fafbe 100644 --- a/frontend/src/components/table/CheckboxTd.tsx +++ b/frontend/src/components/table/CheckboxTd.tsx @@ -8,6 +8,7 @@ type CheckboxTrProps = { onToggle: () => void; isDisabled?: boolean; tooltip?: string; + tdProps?: React.ComponentProps; }; const CheckboxTd: React.FC = ({ @@ -16,6 +17,7 @@ const CheckboxTd: React.FC = ({ onToggle, isDisabled, tooltip, + tdProps, }) => { let content = ( = ({ content = {content}; } - return {content}; + return ( + + {content} + + ); }; export default CheckboxTd; diff --git a/frontend/src/components/table/TableBase.tsx b/frontend/src/components/table/TableBase.tsx index 9c4c6ff695..943123d526 100644 --- a/frontend/src/components/table/TableBase.tsx +++ b/frontend/src/components/table/TableBase.tsx @@ -19,6 +19,7 @@ import { Tbody, Td, TbodyProps, + InnerScrollContainer, } from '@patternfly/react-table'; import { EitherNotBoth } from '~/typeHelpers'; import { GetColumnSort, SortableData } from './types'; @@ -159,6 +160,8 @@ const TableBase = ({ ref={selectAllRef} colSpan={col.colSpan} rowSpan={col.rowSpan} + isStickyColumn={col.isStickyColumn} + stickyMinWidth={col.stickyMinWidth} select={{ isSelected: selectAll.selected, onSelect: (e, value) => selectAll.onSelect(value), @@ -182,6 +185,8 @@ const TableBase = ({ info={col.info} isSubheader={isSubheader} isStickyColumn={col.isStickyColumn} + stickyMinWidth={col.stickyMinWidth} + stickyLeftOffset={col.stickyLeftOffset} hasRightBorder={col.hasRightBorder} modifier={col.modifier} className={col.className} @@ -268,17 +273,23 @@ const TableBase = ({ )} - - {caption && } - - {columns.map((col, i) => renderColumnHeader(col, i))} - {subColumns?.length ? ( - {subColumns.map((col, i) => renderColumnHeader(col, columns.length + i, true))} - ) : null} - - {disableRowRenderSupport ? renderRows() : {renderRows()}} - {footerRow && footerRow(page)} -
{caption}
+ + + + {caption && } + + {columns.map((col, i) => renderColumnHeader(col, i))} + {subColumns?.length ? ( + + {subColumns.map((col, i) => renderColumnHeader(col, columns.length + i, true))} + + ) : null} + + {disableRowRenderSupport ? renderRows() : {renderRows()}} + {footerRow && footerRow(page)} +
{caption}
+ + {!loading && emptyTableView && data.length === 0 && (
{emptyTableView} diff --git a/frontend/src/components/table/types.ts b/frontend/src/components/table/types.ts index b07488647a..8e6bc15586 100644 --- a/frontend/src/components/table/types.ts +++ b/frontend/src/components/table/types.ts @@ -4,7 +4,14 @@ export type GetColumnSort = (columnIndex: number) => ThProps['sort']; export type SortableData = Pick< ThProps, - 'hasRightBorder' | 'isStickyColumn' | 'modifier' | 'width' | 'info' | 'className' + | 'hasRightBorder' + | 'isStickyColumn' + | 'stickyMinWidth' + | 'stickyLeftOffset' + | 'modifier' + | 'width' + | 'info' + | 'className' > & { label: string; field: string; diff --git a/frontend/src/concepts/pipelines/apiHooks/mlmd/__tests__/useGetArtifactsByRuns.spec.ts b/frontend/src/concepts/pipelines/apiHooks/mlmd/__tests__/useGetArtifactsByRuns.spec.ts new file mode 100644 index 0000000000..d49a309d98 --- /dev/null +++ b/frontend/src/concepts/pipelines/apiHooks/mlmd/__tests__/useGetArtifactsByRuns.spec.ts @@ -0,0 +1,155 @@ +import { testHook, standardUseFetchState } from '~/__tests__/unit/testUtils/hooks'; +import { + MetadataStoreServicePromiseClient, + Artifact, + Execution, + Event, + Context, +} from '~/third_party/mlmd'; +import { usePipelinesAPI } from '~/concepts/pipelines/context'; +import { getMlmdContext } from '~/concepts/pipelines/apiHooks/mlmd/useMlmdContext'; +import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; +import { + GetArtifactsByContextResponse, + GetExecutionsByContextResponse, + GetEventsByExecutionIDsResponse, +} from '~/third_party/mlmd/generated/ml_metadata/proto/metadata_store_service_pb'; +import { useGetArtifactsByRuns } from '~/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns'; + +// Mock the usePipelinesAPI and getMlmdContext hooks +jest.mock('~/concepts/pipelines/context', () => ({ + usePipelinesAPI: jest.fn(), +})); + +jest.mock('~/concepts/pipelines/apiHooks/mlmd/useMlmdContext', () => ({ + getMlmdContext: jest.fn(), +})); + +// Mock the MetadataStoreServicePromiseClient +jest.mock('~/third_party/mlmd', () => { + const originalModule = jest.requireActual('~/third_party/mlmd'); + return { + ...originalModule, + MetadataStoreServicePromiseClient: jest.fn().mockImplementation(() => ({ + getArtifactsByContext: jest.fn(), + getExecutionsByContext: jest.fn(), + getEventsByExecutionIDs: jest.fn(), + })), + GetArtifactsByContextRequest: originalModule.GetArtifactsByContextRequest, + GetExecutionsByContextRequest: originalModule.GetExecutionsByContextRequest, + GetEventsByExecutionIDsRequest: originalModule.GetEventsByExecutionIDsRequest, + }; +}); + +describe('useGetArtifactsByRuns', () => { + const mockClient = new MetadataStoreServicePromiseClient(''); + const mockUsePipelinesAPI = jest.mocked( + usePipelinesAPI as () => Partial>, + ); + const mockGetMlmdContext = jest.mocked(getMlmdContext); + const mockGetArtifactsByContext = jest.mocked(mockClient.getArtifactsByContext); + const mockGetExecutionsByContext = jest.mocked(mockClient.getExecutionsByContext); + const mockGetEventsByExecutionIDs = jest.mocked(mockClient.getEventsByExecutionIDs); + + const mockContext = new Context(); + mockContext.setId(1); + + const mockArtifact = new Artifact(); + mockArtifact.setId(1); + mockArtifact.setName('artifact1'); + + const mockExecution = new Execution(); + mockExecution.setId(1); + + const mockEvent = new Event(); + mockEvent.getArtifactId = jest.fn().mockReturnValue(1); + mockEvent.getExecutionId = jest.fn().mockReturnValue(1); + + // eslint-disable-next-line camelcase + const mockRun = { run_id: 'test-run-id' } as PipelineRunKFv2; + + beforeEach(() => { + jest.clearAllMocks(); + mockUsePipelinesAPI.mockReturnValue({ + metadataStoreServiceClient: mockClient, + }); + }); + + it('throws error when no MLMD context is found', async () => { + mockGetMlmdContext.mockResolvedValue(undefined); + const renderResult = testHook(useGetArtifactsByRuns)([mockRun]); + + // wait for update + await renderResult.waitForNextUpdate(); + + expect(renderResult.result.current).toEqual( + standardUseFetchState([], false, new Error('No context for run: test-run-id')), + ); + }); + + it('should fetch and return MLMD packages for pipeline runs', async () => { + mockGetMlmdContext.mockResolvedValue(mockContext); + mockGetArtifactsByContext.mockResolvedValue({ + getArtifactsList: () => [mockArtifact], + } as GetArtifactsByContextResponse); + mockGetExecutionsByContext.mockResolvedValue({ + getExecutionsList: () => [mockExecution], + } as GetExecutionsByContextResponse); + mockGetEventsByExecutionIDs.mockResolvedValue({ + getEventsList: () => [mockEvent], + } as GetEventsByExecutionIDsResponse); + + const renderResult = testHook(useGetArtifactsByRuns)([mockRun]); + + expect(renderResult.result.current).toStrictEqual(standardUseFetchState([])); + expect(renderResult).hookToHaveUpdateCount(1); + + // wait for update + await renderResult.waitForNextUpdate(); + + expect(renderResult.result.current).toStrictEqual( + standardUseFetchState( + [ + { + [mockRun.run_id]: [mockArtifact], + }, + ], + true, + ), + ); + expect(renderResult).hookToHaveUpdateCount(2); + }); + + it('should handle errors from getMlmdContext', async () => { + const error = new Error('Cannot fetch context'); + mockGetMlmdContext.mockRejectedValue(error); + + const renderResult = testHook(useGetArtifactsByRuns)([mockRun]); + + expect(renderResult.result.current).toStrictEqual(standardUseFetchState([])); + expect(renderResult).hookToHaveUpdateCount(1); + + // wait for update + await renderResult.waitForNextUpdate(); + + expect(renderResult.result.current).toStrictEqual(standardUseFetchState([], false, error)); + expect(renderResult).hookToHaveUpdateCount(2); + }); + + it('should handle errors from getArtifactsByContext', async () => { + const error = new Error('Cannot fetch artifacts'); + mockGetMlmdContext.mockResolvedValue(mockContext); + mockGetArtifactsByContext.mockRejectedValue(error); + + const renderResult = testHook(useGetArtifactsByRuns)([mockRun]); + + expect(renderResult.result.current).toStrictEqual(standardUseFetchState([])); + expect(renderResult).hookToHaveUpdateCount(1); + + // wait for update + await renderResult.waitForNextUpdate(); + + expect(renderResult.result.current).toStrictEqual(standardUseFetchState([], false, error)); + expect(renderResult).hookToHaveUpdateCount(2); + }); +}); diff --git a/frontend/src/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns.ts b/frontend/src/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns.ts new file mode 100644 index 0000000000..6ec4d43ca2 --- /dev/null +++ b/frontend/src/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns.ts @@ -0,0 +1,43 @@ +import React from 'react'; + +import { Artifact } from '~/third_party/mlmd'; +import { GetArtifactsByContextRequest } from '~/third_party/mlmd/generated/ml_metadata/proto/metadata_store_service_pb'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { usePipelinesAPI } from '~/concepts/pipelines/context'; +import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; +import { MlmdContextTypes } from './types'; +import { getMlmdContext } from './useMlmdContext'; + +export const useGetArtifactsByRuns = ( + runs: PipelineRunKFv2[], +): FetchState[]> => { + const { metadataStoreServiceClient } = usePipelinesAPI(); + + const call = React.useCallback[]>>( + () => + Promise.all( + runs.map((run) => + getMlmdContext(metadataStoreServiceClient, run.run_id, MlmdContextTypes.RUN).then( + async (context) => { + if (!context) { + throw new Error(`No context for run: ${run.run_id}`); + } + + const request = new GetArtifactsByContextRequest(); + request.setContextId(context.getId()); + + const response = await metadataStoreServiceClient.getArtifactsByContext(request); + const artifacts = response.getArtifactsList(); + + return { + [run.run_id]: artifacts, + }; + }, + ), + ), + ), + [metadataStoreServiceClient, runs], + ); + + return useFetchState(call, []); +}; diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactVisualization.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactVisualization.tsx index a6b8386810..c9903fad86 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactVisualization.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/ArtifactVisualization.tsx @@ -36,6 +36,7 @@ import { extractS3UriComponents } from '~/concepts/pipelines/content/artifacts/u import MarkdownView from '~/components/MarkdownView'; import { useIsAreaAvailable, SupportedArea } from '~/concepts/areas'; import { bytesAsRoundedGiB } from '~/utilities/number'; +import { getArtifactProperties } from './utils'; interface ArtifactVisualizationProps { artifact: Artifact; @@ -116,24 +117,7 @@ export const ArtifactVisualization: React.FC = ({ ar } if (artifactType === ArtifactType.METRICS) { - const scalarMetrics = artifact - .toObject() - .customPropertiesMap.reduce( - ( - acc: { name: string; value: string }[], - [customPropKey, { stringValue, intValue, doubleValue, boolValue }], - ) => { - if (customPropKey !== 'display_name') { - acc.push({ - name: customPropKey, - value: stringValue || (intValue || doubleValue || boolValue).toString(), - }); - } - - return acc; - }, - [], - ); + const artifactProperties = getArtifactProperties(artifact); return ( @@ -141,7 +125,7 @@ export const ArtifactVisualization: React.FC = ({ ar { + const mockArtifact = new Artifact(); + mockArtifact.setId(1); + mockArtifact.setName('artifact-1'); + mockArtifact.getCustomPropertiesMap().set('display_name', new Value()); + + it('returns empty array when no custom props exist other than display_name', () => { + const result = getArtifactProperties(mockArtifact); + expect(result).toEqual([]); + }); + + it('returns artifact properties when custom props exist other than display_name', () => { + mockArtifact + .getCustomPropertiesMap() + .set('metric-string', new Value().setStringValue('some string')); + mockArtifact.getCustomPropertiesMap().set('metric-int', new Value().setIntValue(10)); + mockArtifact.getCustomPropertiesMap().set('metric-double', new Value().setDoubleValue(1.1)); + mockArtifact.getCustomPropertiesMap().set('metric-bool', new Value().setBoolValue(true)); + + const result = getArtifactProperties(mockArtifact); + expect(result).toEqual([ + { name: 'metric-bool', value: 'true' }, + { name: 'metric-double', value: '1.1' }, + { name: 'metric-int', value: '10' }, + { name: 'metric-string', value: 'some string' }, + ]); + }); +}); diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/utils.ts b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/utils.ts new file mode 100644 index 0000000000..0b39bcd1f5 --- /dev/null +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/utils.ts @@ -0,0 +1,26 @@ +import { Artifact } from '~/third_party/mlmd'; + +export interface ArtifactProperty { + name: string; + value: string; +} + +export const getArtifactProperties = (artifact: Artifact): ArtifactProperty[] => + artifact + .toObject() + .customPropertiesMap.reduce( + ( + acc: { name: string; value: string }[], + [customPropKey, { stringValue, intValue, doubleValue, boolValue }], + ) => { + if (customPropKey !== 'display_name') { + acc.push({ + name: customPropKey, + value: stringValue || (intValue || doubleValue || boolValue).toString(), + }); + } + + return acc; + }, + [], + ); diff --git a/frontend/src/concepts/pipelines/content/tables/columns.ts b/frontend/src/concepts/pipelines/content/tables/columns.ts index b232fc6e4a..7f52d3ee45 100644 --- a/frontend/src/concepts/pipelines/content/tables/columns.ts +++ b/frontend/src/concepts/pipelines/content/tables/columns.ts @@ -124,6 +124,53 @@ export const pipelineRunColumns: SortableData[] = [ kebabTableColumn(), ]; +export function getExperimentRunColumns( + metricsColumnNames: string[], +): SortableData[] { + return [ + { ...checkboxTableColumn(), isStickyColumn: true, stickyMinWidth: '45px' }, + { + label: 'Run', + field: 'name', + sortable: true, + isStickyColumn: true, + hasRightBorder: true, + stickyMinWidth: '250px', + stickyLeftOffset: '45px', + }, + { + label: 'Pipeline version', + field: 'pipeline_version', + sortable: false, + width: 15, + }, + { + label: 'Started', + field: 'created_at', + sortable: true, + width: 15, + }, + { + label: 'Duration', + field: 'duration', + sortable: false, + width: 15, + }, + { + label: 'Status', + field: 'status', + sortable: true, + width: 10, + }, + ...metricsColumnNames.map((metricName: string) => ({ + label: metricName, + field: metricName, + sortable: false, + })), + kebabTableColumn(), + ]; +} + export const pipelineRunJobColumns: SortableData[] = [ checkboxTableColumn(), { diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/CustomMetricsColumnsModal.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/CustomMetricsColumnsModal.tsx new file mode 100644 index 0000000000..a3fd4c9717 --- /dev/null +++ b/frontend/src/concepts/pipelines/content/tables/pipelineRun/CustomMetricsColumnsModal.tsx @@ -0,0 +1,181 @@ +import React from 'react'; +import { + Button, + Checkbox, + DataListDragButton, + DragDrop, + Draggable, + DraggableItemPosition, + Droppable, + Flex, + FlexItem, + Label, + Modal, + ModalBoxBody, + ModalVariant, + Stack, + StackItem, + Tooltip, +} from '@patternfly/react-core'; +import { getMetricsColumnsLocalStorageKey } from './utils'; + +interface MetricsColumn { + id: string; + checked: boolean; +} + +interface CustomMetricsColumnsModalProps { + columns: MetricsColumn[]; + experimentId: string | undefined; + onClose: () => void; +} + +export const CustomMetricsColumnsModal: React.FC = ({ + onClose, + experimentId, + ...props +}) => { + const [columns, setColumns] = React.useState(props.columns); + const [dragDropText, setDragDropText] = React.useState(''); + const metricsColumnsLocalStorageKey = getMetricsColumnsLocalStorageKey(experimentId ?? ''); + const selectedColumns = Object.values(columns).reduce((acc: string[], column) => { + if (column.checked) { + acc.push(column.id); + } + return acc; + }, []); + + const onDrag = React.useCallback( + (source: DraggableItemPosition) => { + setDragDropText(`Started dragging ${columns[source.index].id}`); + + return true; + }, + [columns], + ); + + const onDragMove = React.useCallback( + (source: DraggableItemPosition, dest?: DraggableItemPosition | undefined) => { + const newDragDropText = dest + ? `Move ${columns[source.index].id} to ${columns[dest.index].id}` + : 'Invalid drop zone'; + + if (newDragDropText !== dragDropText) { + setDragDropText(newDragDropText); + } + }, + [columns, dragDropText], + ); + + const onDrop = React.useCallback( + (source: DraggableItemPosition, dest?: DraggableItemPosition | undefined) => { + if (dest) { + const reorderedColumns = columns; + const [removedColumns] = reorderedColumns.splice(source.index, 1); + reorderedColumns.splice(dest.index, 0, removedColumns); + + setColumns(reorderedColumns); + setDragDropText('Dragging finished.'); + + return true; + } + + return false; + }, + [columns], + ); + + const onUpdate = React.useCallback(() => { + localStorage.removeItem(metricsColumnsLocalStorageKey); + localStorage.setItem(metricsColumnsLocalStorageKey, JSON.stringify(selectedColumns)); + + onClose(); + }, [metricsColumnsLocalStorageKey, onClose, selectedColumns]); + + return ( + + + Select up to 10 metrics that will display as columns in the table. Drag and drop column + names to reorder them. + + + + + + } + isOpen + onClose={onClose} + actions={[ + , + , + ]} + > + + + + {columns.map(({ id, checked }) => { + const columnCheckbox = ( + + setColumns((prevColumns) => + prevColumns.map((prevColumn) => { + if (prevColumn.id === id) { + return { id, checked: isChecked }; + } + + return prevColumn; + }), + ) + } + /> + ); + + return ( + + + + + {selectedColumns.length === 10 && !checked ? ( + + {columnCheckbox} + + ) : ( + columnCheckbox + )} + + {id} + + + ); + })} + + + + + ); +}; diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx index 57bf1e6f7f..c5ce1f0c35 100644 --- a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx +++ b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTable.tsx @@ -1,11 +1,16 @@ import React from 'react'; import { useNavigate, useParams } from 'react-router-dom'; -import { Button, Tooltip } from '@patternfly/react-core'; -import { TableVariant } from '@patternfly/react-table'; +import { Button, Skeleton, Tooltip } from '@patternfly/react-core'; +import { TableVariant, Td } from '@patternfly/react-table'; +import { ColumnsIcon } from '@patternfly/react-icons'; + import { TableBase, getTableColumnSort, useCheckboxTable } from '~/components/table'; -import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; -import { pipelineRunColumns } from '~/concepts/pipelines/content/tables/columns'; +import { ArtifactType, PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; +import { + getExperimentRunColumns, + pipelineRunColumns, +} from '~/concepts/pipelines/content/tables/columns'; import PipelineRunTableRow from '~/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow'; import DashboardEmptyTableView from '~/concepts/dashboard/DashboardEmptyTableView'; import PipelineRunTableToolbar from '~/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableToolbar'; @@ -21,6 +26,13 @@ import { useSetVersionFilter } from '~/concepts/pipelines/content/tables/useSetV import { createRunRoute, experimentsCompareRunsRoute } from '~/routes'; import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; import { useContextExperimentArchived } from '~/pages/pipelines/global/experiments/ExperimentRunsContext'; +import { + ArtifactProperty, + getArtifactProperties, +} from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/utils'; +import { useGetArtifactsByRuns } from '~/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns'; +import { CustomMetricsColumnsModal } from './CustomMetricsColumnsModal'; +import { getMetricsColumnsLocalStorageKey } from './utils'; type PipelineRunTableProps = { runs: PipelineRunKFv2[]; @@ -38,13 +50,23 @@ type PipelineRunTableProps = { runType: PipelineRunType.ACTIVE | PipelineRunType.ARCHIVED; }; -const PipelineRunTable: React.FC = ({ +interface PipelineRunTableInternalProps extends Omit { + runs: (PipelineRunKFv2 & { metrics: ArtifactProperty[] })[]; + artifactsLoaded: boolean; + artifactsError: Error | undefined; + metricsNames: Set; +} + +const PipelineRunTableInternal: React.FC = ({ runs, loading, totalSize, page, pageSize, runType, + metricsNames, + artifactsLoaded, + artifactsError, setPage, setPageSize, setFilter, @@ -65,6 +87,7 @@ const PipelineRunTable: React.FC = ({ const [isDeleteModalOpen, setIsDeleteModalOpen] = React.useState(false); const [isArchiveModalOpen, setIsArchiveModalOpen] = React.useState(false); const [isRestoreModalOpen, setIsRestoreModalOpen] = React.useState(false); + const [isCustomColModalOpen, setIsCustomColModalOpen] = React.useState(false); const selectedRuns = selectedIds.reduce((acc: PipelineRunKFv2[], selectedId) => { const selectedRun = runs.find((run) => run.run_id === selectedId); @@ -74,9 +97,18 @@ const PipelineRunTable: React.FC = ({ return acc; }, []); - const restoreButtonTooltipRef = React.useRef(null); const isExperimentArchived = useContextExperimentArchived(); + const isExperimentsEnabled = isExperimentsAvailable && experimentId; + const metricsColumnsLocalStorageKey = localStorage.getItem( + getMetricsColumnsLocalStorageKey(experimentId ?? ''), + ); + const metricsColumnNames = metricsColumnsLocalStorageKey + ? JSON.parse(metricsColumnsLocalStorageKey) + : []; + const columns = isExperimentsEnabled + ? getExperimentRunColumns(metricsColumnNames) + : pipelineRunColumns; const primaryToolbarAction = React.useMemo(() => { if (runType === PipelineRunType.ARCHIVED) { @@ -125,7 +157,7 @@ const PipelineRunTable: React.FC = ({ ]); const compareRunsAction = - isExperimentsAvailable && experimentId && !isExperimentArchived ? ( + isExperimentsEnabled && !isExperimentArchived ? ( + ))} /> )} variant={TableVariant.compact} - getColumnSort={getTableColumnSort({ columns: pipelineRunColumns, ...tableProps })} + getColumnSort={getTableColumnSort({ + columns, + ...tableProps, + })} data-testid={`${runType}-runs-table`} id={`${runType}-runs-table`} /> @@ -253,8 +310,55 @@ const PipelineRunTable: React.FC = ({ }} /> )} + {isCustomColModalOpen && ( + ({ + id: metricName, + checked: metricsColumnNames.includes(metricName), + }))} + onClose={() => setIsCustomColModalOpen(false)} + /> + )} ); }; +const PipelineRunTable: React.FC = ({ runs, page, ...props }) => { + const [runArtifacts, runArtifactsLoaded, runArtifactsError] = useGetArtifactsByRuns(runs); + const metricsNames = new Set(); + + const runsWithMetrics = runs.map((run) => ({ + ...run, + metrics: runArtifacts.reduce((acc: ArtifactProperty[], runArtifactseMap) => { + const artifacts = Object.entries(runArtifactseMap).find( + ([runId]) => run.run_id === runId, + )?.[1]; + + artifacts?.forEach((artifact) => { + if (artifact.getType() === ArtifactType.METRICS) { + const artifactProperties = getArtifactProperties(artifact); + + artifactProperties.map((artifactProp) => metricsNames.add(artifactProp.name)); + acc.push(...artifactProperties); + } + }); + + return acc; + }, []), + })); + + return ( + + ); +}; + export default PipelineRunTable; diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow.tsx b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow.tsx index 0569ffaa88..917835b537 100644 --- a/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow.tsx +++ b/frontend/src/concepts/pipelines/content/tables/pipelineRun/PipelineRunTableRow.tsx @@ -27,6 +27,7 @@ type PipelineRunTableRowProps = { checkboxProps: Omit, 'id'>; onDelete?: () => void; run: PipelineRunKFv2; + customCells?: React.ReactNode; hasExperiments?: boolean; hasRowActions?: boolean; }; @@ -35,6 +36,7 @@ const PipelineRunTableRow: React.FC = ({ hasRowActions = true, hasExperiments = true, checkboxProps, + customCells, onDelete, run, }) => { @@ -123,7 +125,16 @@ const PipelineRunTableRow: React.FC = ({ return ( - {hasExperiments && } @@ -144,6 +155,7 @@ const PipelineRunTableRow: React.FC = ({ + {customCells} {hasRowActions && (
+ {!artifactsLoaded && !artifactsError ? ( + + ) : ( + run.metrics.find((metric) => metric.name === metricName)?.value ?? '-' + )} +
+ diff --git a/frontend/src/concepts/pipelines/content/tables/pipelineRun/utils.ts b/frontend/src/concepts/pipelines/content/tables/pipelineRun/utils.ts new file mode 100644 index 0000000000..f18b016c88 --- /dev/null +++ b/frontend/src/concepts/pipelines/content/tables/pipelineRun/utils.ts @@ -0,0 +1,3 @@ +export function getMetricsColumnsLocalStorageKey(experimentId: string): string { + return `metrics-columns-${experimentId}`; +}