-
Notifications
You must be signed in to change notification settings - Fork 178
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[RHOAIENG-7481] Add metrics columns to pipeline run table and modal p…
…aram selector
- Loading branch information
Showing
14 changed files
with
658 additions
and
48 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
155 changes: 155 additions & 0 deletions
155
frontend/src/concepts/pipelines/apiHooks/mlmd/__tests__/useGetArtifactsByRuns.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<ReturnType<typeof usePipelinesAPI>>, | ||
| ); | ||
| 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); | ||
| }); | ||
| }); |
43 changes: 43 additions & 0 deletions
43
frontend/src/concepts/pipelines/apiHooks/mlmd/useGetArtifactsByRuns.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<Record<string, Artifact[]>[]> => { | ||
| const { metadataStoreServiceClient } = usePipelinesAPI(); | ||
|
|
||
| const call = React.useCallback<FetchStateCallbackPromise<Record<string, Artifact[]>[]>>( | ||
| () => | ||
| 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, []); | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31 changes: 31 additions & 0 deletions
31
...concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/__tests__/utils.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { Artifact, Value } from '~/third_party/mlmd'; | ||
| import { getArtifactProperties } from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/artifacts/utils'; | ||
|
|
||
| describe('getArtifactProperties', () => { | ||
| 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' }, | ||
| ]); | ||
| }); | ||
| }); |
Oops, something went wrong.