Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execution reference on the artifact view what execution produced the artifacts #3470

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions frontend/src/__mocks__/mlmd/mockGetEventsByArtifactIDs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { GetEventsByArtifactIDsResponse } from '~/__mocks__/third_party/mlmd';
import createGrpcResponse, { GrpcResponse } from './utils';

const mockedEventsByArtifactIdsResponse: GetEventsByArtifactIDsResponse = {
events: [
{
artifactId: 1,
executionId: 27,
path: {
steps: [
{
key: 'metrics',
},
],
},
type: 4,
millisecondsSinceEpoch: 1712899529364,
},
],
};

export const mockGetEventsByArtifactIDs = (): GrpcResponse => {
const binary = GetEventsByArtifactIDsResponse.encode(mockedEventsByArtifactIdsResponse).finish();
return createGrpcResponse(binary);
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { GrpcResponse } from '~/__mocks__/mlmd/utils';
import { Contextual } from '~/__tests__/cypress/cypress/pages/components/Contextual';
import { TableRow } from '~/__tests__/cypress/cypress/pages/components/table';

class ArtifactsGlobal {
visit(projectName: string) {
Expand Down Expand Up @@ -87,6 +88,12 @@ class ArtifactsTableRow extends Contextual<HTMLTableRowElement> {
}
}

class ReferenceTableRow extends TableRow {
findExecutionLink(name: string) {
return this.find().find(`[data-label=Link]`).contains(name);
}
}

class ArtifactDetails {
visit(projectName: string, artifactName: string, artifactId: string) {
cy.visitWithLogin(`/artifacts/${projectName}/${artifactId}`);
Expand All @@ -102,6 +109,16 @@ class ArtifactDetails {
return cy.findByTestId(`dataset-description-list-${label}`);
}

findReferenceTable() {
return cy.findByTestId('artifact-details-reference-table');
}

getReferenceTableRow() {
return new ReferenceTableRow(() =>
this.findReferenceTable().find(`[data-label=Name]`).contains('Execution').parents('tr'),
);
}

findCustomPropItemByLabel(label: string) {
return cy.findByTestId(`custom-props-description-list-${label}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,11 @@ declare global {
options: { path: { namespace: string; serviceName: string } },
response: OdhResponse<GrpcResponse>,
) => Cypress.Chainable<null>) &
((
type: `POST /api/service/mlmd/:namespace/:serviceName/ml_metadata.MetadataStoreService/GetEventsByArtifactIDs`,
options: { path: { namespace: string; serviceName: string } },
response: OdhResponse<GrpcResponse>,
) => Cypress.Chainable<null>) &
((
type: 'GET /api/rolebindings/opendatahub/openshift-ai-notebooks-image-pullers',
response: OdhResponse<K8sResourceListResult<RoleBindingKind>>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from '~/__mocks__';
import { pipelineRunDetails } from '~/__tests__/cypress/cypress/pages/pipelines';
import { mockArtifactStorage } from '~/__mocks__/mockArtifactStorage';
import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url';
import { configIntercept, dspaIntercepts, projectsIntercept } from './intercepts';
import { initMlmdIntercepts } from './mlmdUtils';

Expand Down Expand Up @@ -178,6 +179,10 @@ describe('Artifacts', () => {
.findCustomPropItemByLabel('display_name')
.next()
.should('have.text', 'scalar metrics');
artifactDetails.findReferenceTable().should('exist');
artifactDetails.getReferenceTableRow().findExecutionLink('execution/211');
artifactDetails.getReferenceTableRow().findExecutionLink('execution/211').click();
verifyRelativeURL('/executions/test-project-name/211');
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Interception } from 'cypress/types/net-stubbing';
import { mockGetEventsByArtifactIDs } from '~/__mocks__/mlmd/mockGetEventsByArtifactIDs';
import { mockGetArtifactTypes } from '~/__mocks__/mlmd/mockGetArtifactTypes';
import { mockGetArtifactsByContext } from '~/__mocks__/mlmd/mockGetArtifactsByContext';
import { mockGetContextByTypeAndName } from '~/__mocks__/mlmd/mockGetContextByTypeAndName';
Expand Down Expand Up @@ -48,6 +49,11 @@ export const initMlmdIntercepts = (
{ path: { namespace: projectName, serviceName: 'dspa' } },
mockGetEventsByExecutionIDs(),
);
cy.interceptOdh(
'POST /api/service/mlmd/:namespace/:serviceName/ml_metadata.MetadataStoreService/GetEventsByArtifactIDs',
{ path: { namespace: projectName, serviceName: 'dspa' } },
mockGetEventsByArtifactIDs(),
);
};

// We remove the first 5 bits of the Uint8Array due to an offset from createGrpcResponse
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { testHook, standardUseFetchState } from '~/__tests__/unit/testUtils/hooks';
import {
MetadataStoreServicePromiseClient,
GetEventsByArtifactIDsResponse,
Event,
} from '~/third_party/mlmd';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { useGetEventByArtifactId } from '~/concepts/pipelines/apiHooks/mlmd/useGetEventByArtifactId';

jest.mock('~/concepts/pipelines/context', () => ({
usePipelinesAPI: jest.fn(),
}));

jest.mock('~/third_party/mlmd', () => {
const originalModule = jest.requireActual('~/third_party/mlmd');
return {
...originalModule,
MetadataStoreServicePromiseClient: jest.fn().mockImplementation(() => ({
getEventsByArtifactIDs: jest.fn(),
})),
GetEventsByArtifactIDsRequest: originalModule.GetEventsByArtifactIDsRequest,
};
});

const mockEvent = new Event();
mockEvent.getArtifactId = jest.fn().mockReturnValue(1);
mockEvent.getExecutionId = jest.fn().mockReturnValue(1);

describe('useGetEventByArtifactId', () => {
const mockClient = new MetadataStoreServicePromiseClient('');
const mockUsePipelinesAPI = jest.mocked(
usePipelinesAPI as () => Partial<ReturnType<typeof usePipelinesAPI>>,
);
const mockedGetEventsByArtifactIDs = jest.mocked(mockClient.getEventsByArtifactIDs);

beforeEach(() => {
jest.clearAllMocks();
mockUsePipelinesAPI.mockReturnValue({
metadataStoreServiceClient: mockClient,
});
});

it('should fetch and return events by artifact ID', async () => {
mockedGetEventsByArtifactIDs.mockResolvedValue({
getEventsList: () => [mockEvent],
} as GetEventsByArtifactIDsResponse);

const renderResult = testHook(useGetEventByArtifactId)(1);

expect(renderResult.result.current).toStrictEqual(standardUseFetchState(null));
expect(renderResult).hookToHaveUpdateCount(1);

//wait for update
await renderResult.waitForNextUpdate();

expect(renderResult.result.current).toStrictEqual(standardUseFetchState(mockEvent, true));
expect(renderResult.result.current[0]?.getExecutionId()).toStrictEqual(1);
expect(renderResult).hookToHaveUpdateCount(2);
const [request] = mockedGetEventsByArtifactIDs.mock.calls[0];
expect(request.getArtifactIdsList()).toContain(1);
});

it('should handle errors', async () => {
const error = new Error('Cannot fetch events');
mockedGetEventsByArtifactIDs.mockRejectedValue(error);

const renderResult = testHook(useGetEventByArtifactId)(1);

expect(renderResult.result.current).toStrictEqual(standardUseFetchState(null));
expect(renderResult).hookToHaveUpdateCount(1);

// wait for update
await renderResult.waitForNextUpdate();

expect(renderResult.result.current).toStrictEqual(standardUseFetchState(null, false, error));
expect(renderResult).hookToHaveUpdateCount(2);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import React from 'react';
import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { GetEventsByArtifactIDsRequest, Event } from '~/third_party/mlmd';

export const useGetEventByArtifactId = (artifactId?: number): FetchState<Event | null> => {
const { metadataStoreServiceClient } = usePipelinesAPI();

const call = React.useCallback<FetchStateCallbackPromise<Event | null>>(async () => {
const request = new GetEventsByArtifactIDsRequest();

if (!artifactId) {
return null;

Check warning on line 13 in frontend/src/concepts/pipelines/apiHooks/mlmd/useGetEventByArtifactId.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/pipelines/apiHooks/mlmd/useGetEventByArtifactId.ts#L13

Added line #L13 was not covered by tests
}

request.setArtifactIdsList([artifactId]);

const response = await metadataStoreServiceClient.getEventsByArtifactIDs(request);
return response.getEventsList().length !== 0 ? response.getEventsList()[0] : null;
}, [artifactId, metadataStoreServiceClient]);

return useFetchState(call, null);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,18 @@ import {
DescriptionListDescription,
TextContent,
Text,
StackItem,
} from '@patternfly/react-core';

import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table';
import { Link } from 'react-router-dom';
import { Artifact } from '~/third_party/mlmd';
import { ArtifactUriLink } from '~/concepts/pipelines/content/artifacts/ArtifactUriLink';
import { useGetEventByArtifactId } from '~/concepts/pipelines/apiHooks/mlmd/useGetEventByArtifactId';
import { executionDetailsRoute } from '~/routes';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { useGetExecutionById } from '~/concepts/pipelines/apiHooks/mlmd/useGetExecutionById';
import { getOriginalExecutionId } from '~/pages/pipelines/global/experiments/executions/utils';
import { ArtifactPropertyDescriptionList } from './ArtifactPropertyDescriptionList';

interface ArtifactOverviewDetailsProps {
Expand All @@ -23,6 +31,11 @@ interface ArtifactOverviewDetailsProps {

export const ArtifactOverviewDetails: React.FC<ArtifactOverviewDetailsProps> = ({ artifact }) => {
const artifactObject = artifact?.toObject();
const [event] = useGetEventByArtifactId(artifact?.getId());
const [execution] = useGetExecutionById(event?.getExecutionId().toString());
const { namespace } = usePipelinesAPI();
const originalExecutionId = getOriginalExecutionId(execution);

return (
<Flex
spaceItems={{ default: 'spaceItems2xl' }}
Expand All @@ -48,6 +61,45 @@ export const ArtifactOverviewDetails: React.FC<ArtifactOverviewDetailsProps> = (
</DescriptionList>
</Stack>
</FlexItem>
{execution && (
<FlexItem>
<Stack hasGutter>
<StackItem>
<Title headingLevel="h3">Reference</Title>
</StackItem>
<StackItem>
<Table
aria-label="Artifact details reference table"
data-testid="artifact-details-reference-table"
variant="compact"
borders={false}
>
<Thead>
<Tr>
<Th width={30}>Name</Th>
<Th width={70}>Link</Th>
</Tr>
</Thead>
<Tbody>
<Tr>
<Td dataLabel="Name">Execution</Td>
<Td dataLabel="Link">
<Link
to={executionDetailsRoute(
namespace,
originalExecutionId || execution.getId().toString(),
)}
>
{`execution/${originalExecutionId || execution.getId()}`}
</Link>
</Td>
</Tr>
</Tbody>
</Table>
</StackItem>
</Stack>
</FlexItem>
)}
<FlexItem data-testid="artifact-properties-section">
<Stack hasGutter>
<Title headingLevel="h3">Properties</Title>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useGetPipelineRunContextByExecution } from '~/concepts/pipelines/apiHoo
import { executionDetailsRoute, experimentRunDetailsRoute } from '~/routes';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import usePipelineRunById from '~/concepts/pipelines/apiHooks/usePipelineRunById';
import { getOriginalExecutionId } from '~/pages/pipelines/global/experiments/executions/utils';

type ExecutionDetailsReferenceSectionProps = {
execution: Execution;
Expand All @@ -18,11 +19,7 @@ const ExecutionDetailsReferenceSection: React.FC<ExecutionDetailsReferenceSectio
const { namespace } = usePipelinesAPI();
const [context] = useGetPipelineRunContextByExecution(execution);
const [run, runLoaded, runError] = usePipelineRunById(context?.getName());

const originalExecutionId = execution
.getCustomPropertiesMap()
.get('cached_execution_id')
?.getStringValue();
const originalExecutionId = getOriginalExecutionId(execution);

return (
<Stack hasGutter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ export const getMlmdMetadataValue = (value?: MlmdValue): MlmdMetadataValueType =
}
};

export const getOriginalExecutionId = (execution: Execution | null): string | undefined =>
execution?.getCustomPropertiesMap().get('cached_execution_id')?.getStringValue();

export const parseEventsByType = (response: Event[] | null): Record<Event.Type, Event[]> => {
const events: Record<Event.Type, Event[]> = {
[Event.Type.UNKNOWN]: [],
Expand Down