Skip to content

Commit

Permalink
Merge pull request #2774 from dpanshug/model-version-view
Browse files Browse the repository at this point in the history
Model version view
  • Loading branch information
openshift-merge-bot[bot] authored May 3, 2024
2 parents 0caa9d3 + 8de8532 commit 339060a
Show file tree
Hide file tree
Showing 20 changed files with 584 additions and 84 deletions.
6 changes: 3 additions & 3 deletions frontend/src/__mocks__/mockModelVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { ModelVersion, ModelVersionState } from '~/concepts/modelRegistry/types'

type MockModelVersionType = {
author?: string;
registeredModelID?: string;
registeredModelId?: string;
};

export const mockModelVersion = ({
author = 'Test author',
registeredModelID = '1',
registeredModelId = '1',
}: MockModelVersionType): ModelVersion => ({
author,
createTimeSinceEpoch: '1712234877179',
Expand All @@ -16,5 +16,5 @@ export const mockModelVersion = ({
lastUpdateTimeSinceEpoch: '1712234877179',
name: 'fraud detection model version 1',
state: ModelVersionState.ARCHIVED,
registeredModelID,
registeredModelId,
});
2 changes: 1 addition & 1 deletion frontend/src/__mocks__/mockModelVersionList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ModelVersionList } from '~/concepts/modelRegistry/types';
import { mockModelVersion } from './mockModelVersion';

export const mockModelVersionList = (): ModelVersionList => ({
items: [mockModelVersion({ author: 'Author 1', registeredModelID: '1' })],
items: [mockModelVersion({ author: 'Author 1', registeredModelId: '1' })],
nextPageToken: '',
pageSize: 0,
size: 1,
Expand Down
20 changes: 18 additions & 2 deletions frontend/src/api/modelRegistry/__tests__/custom.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
patchModelArtifact,
patchModelVersion,
patchRegisteredModel,
getModelArtifactsByModelVersion,
} from '~/api/modelRegistry/custom';
import { MODEL_REGISTRY_API_VERSION } from '~/concepts/modelRegistry/const';

Expand Down Expand Up @@ -80,7 +81,7 @@ describe('createModelVersion', () => {
description: 'test',
externalID: '1',
author: 'test author',
registeredModelID: '1',
registeredModelId: '1',
name: 'test new model version',
state: ModelVersionState.LIVE,
customProperties: {},
Expand All @@ -94,7 +95,7 @@ describe('createModelVersion', () => {
description: 'test',
externalID: '1',
author: 'test author',
registeredModelID: '1',
registeredModelId: '1',
name: 'test new model version',
state: ModelVersionState.LIVE,
customProperties: {},
Expand Down Expand Up @@ -254,6 +255,21 @@ describe('getModelVersionsByRegisteredModel', () => {
});
});

describe('getModelArtifactsByModelVersion', () => {
it('should call proxyGET and handleModelRegistryFailures to list models artifacts by model version', () => {
expect(getModelArtifactsByModelVersion('hostPath')({}, '1')).toBe(mockResultPromise);
expect(proxyGETMock).toHaveBeenCalledTimes(1);
expect(proxyGETMock).toHaveBeenCalledWith(
'hostPath',
`/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/1/artifacts`,
{},
K8sAPIOptionsMock,
);
expect(handleModelRegistryFailuresMock).toHaveBeenCalledTimes(1);
expect(handleModelRegistryFailuresMock).toHaveBeenCalledWith(mockProxyPromise);
});
});

describe('patchRegisteredModel', () => {
it('should call proxyPATCH and handleModelRegistryFailures to update registered model', () => {
expect(
Expand Down
20 changes: 16 additions & 4 deletions frontend/src/api/modelRegistry/custom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ export const createModelArtifact =

export const getRegisteredModel =
(hostPath: string) =>
(opts: K8sAPIOptions, registeredModelID: string): Promise<RegisteredModel> =>
(opts: K8sAPIOptions, registeredModelId: string): Promise<RegisteredModel> =>
handleModelRegistryFailures(
proxyGET(
hostPath,
`/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredModelID}`,
`/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredModelId}`,
{},
opts,
),
Expand Down Expand Up @@ -137,17 +137,29 @@ export const getModelVersionsByRegisteredModel =
),
);

export const getModelArtifactsByModelVersion =
(hostpath: string) =>
(opts: K8sAPIOptions, modelVersionId: string): Promise<ModelArtifactList> =>
handleModelRegistryFailures(
proxyGET(
hostpath,
`/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/${modelVersionId}/artifacts`,
{},
opts,
),
);

export const patchRegisteredModel =
(hostPath: string) =>
(
opts: K8sAPIOptions,
data: Partial<RegisteredModel>,
registeredModelID: string,
registeredModelId: string,
): Promise<RegisteredModel> =>
handleModelRegistryFailures(
proxyPATCH(
hostPath,
`/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredModelID}`,
`/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/${registeredModelId}`,
data,
opts,
),
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/components/DashboardDescriptionListGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
DescriptionListTerm,
Split,
SplitItem,
Text,
} from '@patternfly/react-core';
import text from '@patternfly/react-styles/css/utilities/Text/text';
import { CheckIcon, PencilAltIcon, TimesIcon } from '@patternfly/react-icons';
Expand Down Expand Up @@ -99,7 +100,13 @@ const DashboardDescriptionListGroup: React.FC<DashboardDescriptionListGroupProps
<DescriptionListTerm>{title}</DescriptionListTerm>
)}
<DescriptionListDescription className={isEmpty && !isEditing ? text.disabledColor_100 : ''}>
{isEditing ? contentWhenEditing : isEmpty ? contentWhenEmpty : children}
{isEditing ? (
contentWhenEditing
) : isEmpty ? (
<Text style={{ color: '--pf-v5-global--Color--200' }}>{contentWhenEmpty}</Text>
) : (
children
)}
</DescriptionListDescription>
</DescriptionListGroup>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as React from 'react';
import useFetchState, {
FetchState,
FetchStateCallbackPromise,
NotReadyError,
} from '~/utilities/useFetchState';
import { ModelArtifactList } from '~/concepts/modelRegistry/types';
import { useModelRegistryAPI } from '~/concepts/modelRegistry/context/ModelRegistryContext';

const useModelArtifactsByVersionId = (modelVersionId?: string): FetchState<ModelArtifactList> => {
const { api } = useModelRegistryAPI();
const callback = React.useCallback<FetchStateCallbackPromise<ModelArtifactList>>(
(opts) => {
if (!modelVersionId) {
return Promise.reject(new NotReadyError('No model registeredModel id'));
}
return api.getModelArtifactsByModelVersion(opts, modelVersionId);
},
[api, modelVersionId],
);

return useFetchState(
callback,
{ items: [], size: 0, pageSize: 0, nextPageToken: '' },
{ initialPromisePurity: true },
);
};

export default useModelArtifactsByVersionId;
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getListModelVersions,
getListRegisteredModels,
getModelArtifact,
getModelArtifactsByModelVersion,
getModelVersion,
getModelVersionsByRegisteredModel,
getRegisteredModel,
Expand All @@ -35,6 +36,7 @@ const useModelRegistryAPIState = (
listModelVersions: getListModelVersions(path),
listRegisteredModels: getListRegisteredModels(path),
getModelVersionsByRegisteredModel: getModelVersionsByRegisteredModel(path),
getModelArtifactsByModelVersion: getModelArtifactsByModelVersion(path),
patchRegisteredModel: patchRegisteredModel(path),
patchModelVersion: patchModelVersion(path),
patchModelArtifact: patchModelArtifact(path),
Expand Down
12 changes: 9 additions & 3 deletions frontend/src/concepts/modelRegistry/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export type ModelArtifact = ModelRegistryBase & {
export type ModelVersion = ModelRegistryBase & {
state?: ModelVersionState;
author?: string;
registeredModelID: string;
registeredModelId: string;
};

export type RegisteredModel = ModelRegistryBase & {
Expand Down Expand Up @@ -131,7 +131,7 @@ export type CreateModelArtifact = (

export type GetRegisteredModel = (
opts: K8sAPIOptions,
registeredModelID: string,
registeredModelId: string,
) => Promise<RegisteredModel>;

export type GetModelVersion = (
Expand All @@ -155,10 +155,15 @@ export type GetModelVersionsByRegisteredModel = (
registeredmodelId: string,
) => Promise<ModelVersionList>;

export type GetModelArtifactsByModelVersion = (
opts: K8sAPIOptions,
modelVersionId: string,
) => Promise<ModelArtifactList>;

export type PatchRegisteredModel = (
opts: K8sAPIOptions,
data: Partial<RegisteredModel>,
registeredModelID: string,
registeredModelId: string,
) => Promise<RegisteredModel>;

export type PatchModelVersion = (
Expand All @@ -184,6 +189,7 @@ export type ModelRegistryAPIs = {
listModelVersions: GetListModelVersions;
listRegisteredModels: GetListRegisteredModels;
getModelVersionsByRegisteredModel: GetModelVersionsByRegisteredModel;
getModelArtifactsByModelVersion: GetModelArtifactsByModelVersion;
patchRegisteredModel: PatchRegisteredModel;
patchModelVersion: PatchModelVersion;
patchModelArtifact: PatchModelArtifact;
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/pages/modelRegistry/ModelRegistryRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import ModelRegistryCoreLoader from './ModelRegistryCoreLoader';
import ModelRegistry from './screens/ModelRegistry';
import { ModelVersionsTabs } from './screens/const';
import ModelVersions from './screens/ModelVersions';
import ModelVersionsDetails from './screens/ModelVersionDetails/ModelVersionDetails';
import { ModelVersionDetailsTab } from './screens/ModelVersionDetails/const';

const ModelRegistryRoutes: React.FC = () => (
<ModelRegistrySelectorContextProvider>
Expand All @@ -28,6 +30,14 @@ const ModelRegistryRoutes: React.FC = () => (
path={ModelVersionsTabs.DETAILS}
element={<ModelVersions tab={ModelVersionsTabs.DETAILS} empty={false} />}
/>
<Route path="versions/:modelVersionId">
<Route index element={<Navigate to={ModelVersionDetailsTab.DETAILS} />} />
<Route
path={ModelVersionDetailsTab.DETAILS}
element={<ModelVersionsDetails tab={ModelVersionDetailsTab.DETAILS} empty={false} />}
/>
<Route path="*" element={<Navigate to="." />} />
</Route>
<Route path="*" element={<Navigate to="." />} />
</Route>
<Route path="*" element={<Navigate to="." />} />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import React from 'react';
import { useNavigate, useParams } from 'react-router';
import { Breadcrumb, BreadcrumbItem, Flex, FlexItem } from '@patternfly/react-core';
import { Link } from 'react-router-dom';
import ApplicationsPage from '~/pages/ApplicationsPage';
import useModelVersionById from '~/concepts/modelRegistry/apiHooks/useModelVersionById';
import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/ModelRegistrySelectorContext';
import { modelVersionUrl, registeredModelUrl } from '~/pages/modelRegistry/screens/routeUtils';
import useRegisteredModelById from '~/concepts/modelRegistry/apiHooks/useRegisteredModelById';
import { ModelVersionDetailsTab } from './const';
import ModelVersionsDetailsHeaderActions from './ModelVersionDetailsHeaderActions';
import ModelVersionDetailsTabs from './ModelVersionDetailsTabs';
import ModelVersionSelector from './ModelVersionSelector';

type ModelVersionsDetailProps = {
tab: ModelVersionDetailsTab;
} & Omit<
React.ComponentProps<typeof ApplicationsPage>,
'breadcrumb' | 'title' | 'description' | 'loadError' | 'loaded' | 'provideChildrenPadding'
>;

const ModelVersionsDetails: React.FC<ModelVersionsDetailProps> = ({ tab, ...pageProps }) => {
const navigate = useNavigate();

const { preferredModelRegistry } = React.useContext(ModelRegistrySelectorContext);

const { modelVersionId: mvId, registeredModelId: rmId } = useParams();
const [rm] = useRegisteredModelById(rmId);
const [mv, mvLoaded, mvLoadError] = useModelVersionById(mvId);

return (
<ApplicationsPage
{...pageProps}
breadcrumb={
<Breadcrumb>
<BreadcrumbItem
render={() => (
<Link to="/modelRegistry">
Registered models - {preferredModelRegistry?.metadata.name}
</Link>
)}
/>
<BreadcrumbItem
render={() => (
<Link to={registeredModelUrl(rmId, preferredModelRegistry?.metadata.name)}>
{rm?.name}
</Link>
)}
/>
<BreadcrumbItem isActive>{mv?.name}</BreadcrumbItem>
</Breadcrumb>
}
title={mv?.name}
headerAction={
mvLoaded &&
mv && (
<Flex
spaceItems={{ default: 'spaceItemsMd' }}
alignItems={{ default: 'alignItemsFlexStart' }}
>
<FlexItem style={{ width: '300px' }}>
<ModelVersionSelector
rmId={rmId}
selection={mv}
onSelect={(modelVersionId) =>
navigate(
modelVersionUrl(modelVersionId, rmId, preferredModelRegistry?.metadata.name),
)
}
/>
</FlexItem>
<FlexItem>
<ModelVersionsDetailsHeaderActions />
</FlexItem>
</Flex>
)
}
description={mv?.description}
loadError={mvLoadError}
loaded={mvLoaded}
provideChildrenPadding
>
{mv !== null && <ModelVersionDetailsTabs tab={tab} modelVersion={mv} />}
</ApplicationsPage>
);
};

export default ModelVersionsDetails;
Loading

0 comments on commit 339060a

Please sign in to comment.