From cc6455f5b31d7aa037148b3c0e1e786550ad1ad0 Mon Sep 17 00:00:00 2001 From: Lucas Fernandez Date: Thu, 12 Sep 2024 23:22:21 +0200 Subject: [PATCH] Set up model registry context and apiHooks (#360) * Set up model registry context and apiHooks Signed-off-by: lucferbux * Add selector core loader Signed-off-by: lucferbux --------- Signed-off-by: lucferbux --- clients/ui/bff/README.md | 24 +- clients/ui/bff/api/app.go | 7 +- clients/ui/frontend/config/webpack.dev.js | 1 - .../src/__mocks__/mockModelArtifact.ts | 34 +++ .../src/__mocks__/mockModelRegistry.ts | 17 ++ .../__mocks__/mockModelRegistryResponse.ts | 8 + .../src/__mocks__/mockModelVersion.ts | 36 +++ .../src/__mocks__/mockModelVersionList.ts | 11 + .../src/__mocks__/mockRegisteredModelsList.ts | 53 ++++ .../cypress/cypress/pages/appChrome.ts | 45 +++ .../__tests__/cypress/cypress/pages/home.ts | 11 - .../cypress/cypress/pages/modelRegistry.ts | 192 +++++++++++++ .../modelRegistryView/registerModelPage.ts | 61 ++++ .../cypress/cypress/support/commands/api.ts | 138 +++++++++ .../cypress/support/commands/application.ts | 271 ++++++++++++++++++ .../cypress/cypress/support/commands/index.ts | 2 + .../cypress/tests/mocked/application.cy.ts | 13 - .../cypress/tests/mocked/modelRegistry.cy.ts | 226 +++++++++++++++ clients/ui/frontend/src/app/App.tsx | 5 +- clients/ui/frontend/src/app/AppRoutes.tsx | 9 +- .../src/app/api/__tests__/errorUtils.spec.ts | 8 +- .../src/app/api/__tests__/service.spec.ts | 204 +++++++------ clients/ui/frontend/src/app/api/apiUtils.ts | 12 +- clients/ui/frontend/src/app/api/errorUtils.ts | 7 +- clients/ui/frontend/src/app/api/k8s.ts | 17 +- clients/ui/frontend/src/app/api/service.ts | 179 +++--------- clients/ui/frontend/src/app/api/types.ts | 19 ++ .../ui/frontend/src/app/api/useAPIState.ts | 32 +++ .../app/components/EmptyStateErrorMessage.tsx | 34 +++ .../src/app/context/ModelRegistryContext.tsx | 44 +++ .../context/ModelRegistrySelectorContext.tsx | 58 ++++ .../app/context/useModelRegistryAPIState.tsx | 54 ++++ .../useModelArtifactsByVersionId.spec.ts | 89 ++++++ .../app/hooks/useModelArtifactsByVersionId.ts | 27 ++ .../src/app/hooks/useModelRegistries.ts | 15 + .../src/app/hooks/useModelRegistryAPI.ts | 16 ++ .../src/app/hooks/useModelVersionById.ts | 26 ++ .../useModelVersionsByRegisteredModel.ts | 32 +++ .../src/app/hooks/useRegisteredModelById.ts | 26 ++ .../src/app/hooks/useRegisteredModels.ts | 24 ++ .../modelRegistry/ModelRegistryCoreLoader.tsx | 137 +++++++++ .../modelRegistry/ModelRegistryRoutes.tsx | 15 +- .../screens/InvalidModelRegistry.tsx | 25 ++ .../{ => screens}/ModelRegistry.tsx | 16 +- .../screens/ModelRegistrySelector.tsx | 196 +++++++++++++ .../ModelRegistrySelectorNavigator.tsx | 27 ++ .../components/EmptyModelRegistryState.tsx | 73 +++++ .../pages/modelRegistry/screens/routeUtils.ts | 51 ++++ clients/ui/frontend/src/app/types.ts | 7 +- clients/ui/frontend/src/types.ts | 11 - .../frontend/src/utilities/useFetchState.ts | 2 +- 51 files changed, 2341 insertions(+), 306 deletions(-) create mode 100644 clients/ui/frontend/src/__mocks__/mockModelArtifact.ts create mode 100644 clients/ui/frontend/src/__mocks__/mockModelRegistry.ts create mode 100644 clients/ui/frontend/src/__mocks__/mockModelRegistryResponse.ts create mode 100644 clients/ui/frontend/src/__mocks__/mockModelVersion.ts create mode 100644 clients/ui/frontend/src/__mocks__/mockModelVersionList.ts create mode 100644 clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts create mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts delete mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/pages/home.ts create mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts create mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts create mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts create mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts delete mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts create mode 100644 clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry.cy.ts create mode 100644 clients/ui/frontend/src/app/api/types.ts create mode 100644 clients/ui/frontend/src/app/api/useAPIState.ts create mode 100644 clients/ui/frontend/src/app/components/EmptyStateErrorMessage.tsx create mode 100644 clients/ui/frontend/src/app/context/ModelRegistryContext.tsx create mode 100644 clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx create mode 100644 clients/ui/frontend/src/app/context/useModelRegistryAPIState.tsx create mode 100644 clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts create mode 100644 clients/ui/frontend/src/app/hooks/useModelArtifactsByVersionId.ts create mode 100644 clients/ui/frontend/src/app/hooks/useModelRegistries.ts create mode 100644 clients/ui/frontend/src/app/hooks/useModelRegistryAPI.ts create mode 100644 clients/ui/frontend/src/app/hooks/useModelVersionById.ts create mode 100644 clients/ui/frontend/src/app/hooks/useModelVersionsByRegisteredModel.ts create mode 100644 clients/ui/frontend/src/app/hooks/useRegisteredModelById.ts create mode 100644 clients/ui/frontend/src/app/hooks/useRegisteredModels.ts create mode 100644 clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx create mode 100644 clients/ui/frontend/src/app/pages/modelRegistry/screens/InvalidModelRegistry.tsx rename clients/ui/frontend/src/app/pages/modelRegistry/{ => screens}/ModelRegistry.tsx (63%) create mode 100644 clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx create mode 100644 clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx create mode 100644 clients/ui/frontend/src/app/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx create mode 100644 clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts diff --git a/clients/ui/bff/README.md b/clients/ui/bff/README.md index 5a566e4a..24c27384 100644 --- a/clients/ui/bff/README.md +++ b/clients/ui/bff/README.md @@ -57,10 +57,10 @@ make docker-build | URL Pattern | Handler | Action | |------------------------------------------------------------------------------------|-------------------------|----------------------------------------------| | GET /v1/healthcheck | HealthcheckHandler | Show application information. | -| GET /v1/model-registry | ModelRegistryHandler | Get all model registries, | -| GET /v1/model-registry/{model_registry_id}/registered_models | RegisteredModelsHandler | Gets a list of all RegisteredModel entities. | -| POST /v1/model-registry/{model_registry_id}/registered_models | RegisteredModelsHandler | Create a RegisteredModel entity. | -| GET /v1/model-registry/{model_registry_id}/registered_models/{registered_model_id} | RegisteredModelHandler | Get a RegisteredModel entity by ID | +| GET /v1/model_registry | ModelRegistryHandler | Get all model registries, | +| GET /v1/model_registry/{model_registry_id}/registered_models | RegisteredModelsHandler | Gets a list of all RegisteredModel entities. | +| POST /v1/model_registry/{model_registry_id}/registered_models | RegisteredModelsHandler | Create a RegisteredModel entity. | +| GET /v1/model_registry/{model_registry_id}/registered_models/{registered_model_id} | RegisteredModelHandler | Get a RegisteredModel entity by ID | ### Sample local calls ``` @@ -68,16 +68,16 @@ make docker-build curl -i localhost:4000/api/v1/healthcheck ``` ``` -# GET /v1/model-registry -curl -i localhost:4000/api/v1/model-registry +# GET /v1/model_registry +curl -i localhost:4000/api/v1/model_registry ``` ``` -# GET /v1/model-registry/{model_registry_id}/registered_models -curl -i localhost:4000/api/v1/model-registry/model-registry/registered_models +# GET /v1/model_registry/{model_registry_id}/registered_models +curl -i localhost:4000/api/v1/model_registry/model_registry_1/registered_models ``` ``` -#POST /v1/model-registry/{model_registry_id}/registered_models -curl -i -X POST "http://localhost:4000/api/v1/model-registry/model-registry/registered_models" \ +#POST /v1/model_registry/{model_registry_id}/registered_models +curl -i -X POST "http://localhost:4000/api/v1/model_registry/model_registry/registered_models" \ -H "Content-Type: application/json" \ -d '{ "customProperties": { @@ -94,6 +94,6 @@ curl -i -X POST "http://localhost:4000/api/v1/model-registry/model-registry/regi }' ``` ``` -# GET /v1/model-registry/{model_registry_id}/registered_models/{registered_model_id} -curl -i localhost:4000/api/v1/model-registry/model-registry/registered_models/1 +# GET /v1/model_registry/{model_registry_id}/registered_models/{registered_model_id} +curl -i localhost:4000/api/v1/model_registry/model_registry/registered_models/1 ``` diff --git a/clients/ui/bff/api/app.go b/clients/ui/bff/api/app.go index 41d1a52a..4040c2ef 100644 --- a/clients/ui/bff/api/app.go +++ b/clients/ui/bff/api/app.go @@ -2,13 +2,14 @@ package api import ( "fmt" + "log/slog" + "net/http" + "github.com/julienschmidt/httprouter" "github.com/kubeflow/model-registry/ui/bff/config" "github.com/kubeflow/model-registry/ui/bff/data" "github.com/kubeflow/model-registry/ui/bff/integrations" "github.com/kubeflow/model-registry/ui/bff/internals/mocks" - "log/slog" - "net/http" ) const ( @@ -17,7 +18,7 @@ const ( ModelRegistryId = "model_registry_id" RegisteredModelId = "registered_model_id" HealthCheckPath = PathPrefix + "/healthcheck" - ModelRegistry = PathPrefix + "/model-registry" + ModelRegistry = PathPrefix + "/model_registry" RegisteredModelsPath = ModelRegistry + "/:" + ModelRegistryId + "/registered_models" RegisteredModelPath = RegisteredModelsPath + "/:" + RegisteredModelId ) diff --git a/clients/ui/frontend/config/webpack.dev.js b/clients/ui/frontend/config/webpack.dev.js index f7a6c806..b2198a83 100644 --- a/clients/ui/frontend/config/webpack.dev.js +++ b/clients/ui/frontend/config/webpack.dev.js @@ -18,7 +18,6 @@ module.exports = merge(common('development'), { host: HOST, port: PORT, historyApiFallback: true, - open: true, static: { directory: path.resolve(relativeDir, 'dist'), }, diff --git a/clients/ui/frontend/src/__mocks__/mockModelArtifact.ts b/clients/ui/frontend/src/__mocks__/mockModelArtifact.ts new file mode 100644 index 00000000..8f2bb628 --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockModelArtifact.ts @@ -0,0 +1,34 @@ +import { ModelArtifact, ModelArtifactState } from '~/app/types'; + +type MockModelArtifact = { + id?: string; + name?: string; + uri?: string; + state?: ModelArtifactState; + author?: string; +}; + +export const mockModelArtifact = ({ + id = '1', + name = 'test', + uri = 'test', + state = ModelArtifactState.LIVE, + author = 'Author 1', +}: MockModelArtifact): ModelArtifact => ({ + id, + name, + externalID: '1234132asdfasdf', + description: '', + createTimeSinceEpoch: '1710404288975', + lastUpdateTimeSinceEpoch: '1710404288975', + customProperties: {}, + uri, + state, + author, + modelFormatName: 'test', + storageKey: 'test', + storagePath: 'test', + modelFormatVersion: 'test', + serviceAccountName: 'test', + artifactType: 'test', +}); diff --git a/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts b/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts new file mode 100644 index 00000000..56fed2e3 --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockModelRegistry.ts @@ -0,0 +1,17 @@ +import { ModelRegistry } from '~/app/types'; + +type MockModelRegistry = { + name?: string; + description?: string; + displayName?: string; +}; + +export const mockModelRegistry = ({ + name = 'modelregistry-sample', + description = 'New model registry', + displayName = 'Model Registry Sample', +}: MockModelRegistry): ModelRegistry => ({ + name, + description, + displayName, +}); diff --git a/clients/ui/frontend/src/__mocks__/mockModelRegistryResponse.ts b/clients/ui/frontend/src/__mocks__/mockModelRegistryResponse.ts new file mode 100644 index 00000000..79b1e197 --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockModelRegistryResponse.ts @@ -0,0 +1,8 @@ +/* eslint-disable camelcase */ +import { ModelRegistryResponse } from '~/app/types'; + +export const mockModelRegistryResponse = ({ + model_registry = [], +}: Partial): ModelRegistryResponse => ({ + model_registry, +}); diff --git a/clients/ui/frontend/src/__mocks__/mockModelVersion.ts b/clients/ui/frontend/src/__mocks__/mockModelVersion.ts new file mode 100644 index 00000000..80a6f310 --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockModelVersion.ts @@ -0,0 +1,36 @@ +import { ModelVersion, ModelState } from '~/app/types'; +import { createModelRegistryLabelsObject } from './utils'; + +type MockModelVersionType = { + author?: string; + id?: string; + registeredModelId?: string; + name?: string; + labels?: string[]; + state?: ModelState; + description?: string; + createTimeSinceEpoch?: string; + lastUpdateTimeSinceEpoch?: string; +}; + +export const mockModelVersion = ({ + author = 'Test author', + registeredModelId = '1', + name = 'new model version', + labels = [], + id = '1', + state = ModelState.LIVE, + description = 'Description of model version', + createTimeSinceEpoch = '1712234877179', + lastUpdateTimeSinceEpoch = '1712234877179', +}: MockModelVersionType): ModelVersion => ({ + author, + createTimeSinceEpoch, + customProperties: createModelRegistryLabelsObject(labels), + id, + lastUpdateTimeSinceEpoch, + name, + state, + registeredModelId, + description, +}); diff --git a/clients/ui/frontend/src/__mocks__/mockModelVersionList.ts b/clients/ui/frontend/src/__mocks__/mockModelVersionList.ts new file mode 100644 index 00000000..16a83379 --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockModelVersionList.ts @@ -0,0 +1,11 @@ +/* eslint-disable camelcase */ +import { ModelVersionList } from '~/app/types'; + +export const mockModelVersionList = ({ + items = [], +}: Partial): ModelVersionList => ({ + items, + nextPageToken: '', + pageSize: 0, + size: items.length, +}); diff --git a/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts b/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts new file mode 100644 index 00000000..ddb525af --- /dev/null +++ b/clients/ui/frontend/src/__mocks__/mockRegisteredModelsList.ts @@ -0,0 +1,53 @@ +import { RegisteredModelList } from '~/app/types'; +import { mockRegisteredModel } from './mockRegisteredModel'; + +export const mockRegisteredModelList = ({ + size = 5, + items = [ + mockRegisteredModel({ name: 'test-1' }), + mockRegisteredModel({ name: 'test-2' }), + mockRegisteredModel({ + name: 'Fraud detection model', + description: + 'A machine learning model trained to detect fraudulent transactions in financial data', + labels: [ + 'Financial data', + 'Fraud detection', + 'Test label', + 'Machine learning', + 'Next data to be overflow', + ], + }), + mockRegisteredModel({ + name: 'Credit Scoring', + labels: [ + 'Credit Score Predictor', + 'Creditworthiness scoring system', + 'Default Risk Analyzer', + 'Portfolio Management', + 'Risk Assessment', + ], + }), + mockRegisteredModel({ + name: 'Label modal', + description: + 'A machine learning model trained to detect fraudulent transactions in financial data', + labels: [ + 'Testing label', + 'Financial data', + 'Fraud detection', + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc', + 'Machine learning', + 'Next data to be overflow', + 'Label x', + 'Label y', + 'Label z', + ], + }), + ], +}: Partial): RegisteredModelList => ({ + items, + nextPageToken: '', + pageSize: 0, + size, +}); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts new file mode 100644 index 00000000..8d30c9a4 --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/appChrome.ts @@ -0,0 +1,45 @@ +class AppChrome { + visit() { + cy.visit('/'); + this.wait(); + } + + private wait() { + cy.get('#dashboard-page-main'); + cy.testA11y(); + } + + // TODO: implement when authorization is enabled + // shouldBeUnauthorized() { + // cy.findByTestId('unauthorized-error'); + // return this; + // } + + findNavToggle() { + return cy.get('#page-nav-toggle'); + } + + findSideBar() { + return cy.get('#page-sidebar'); + } + + findNavSection(name: string) { + return this.findSideBar().findByRole('button', { name }); + } + + findNavItem(name: string, section?: string) { + if (section) { + this.findNavSection(section) + // do not fail if the section is not found + .should('have.length.at.least', 0) + .then(($el) => { + if ($el.attr('aria-expanded') === 'false') { + cy.wrap($el).click(); + } + }); + } + return this.findSideBar().findByRole('link', { name }); + } +} + +export const appChrome = new AppChrome(); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/home.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/home.ts deleted file mode 100644 index 4299e844..00000000 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/home.ts +++ /dev/null @@ -1,11 +0,0 @@ -class Home { - visit() { - cy.visit(`/`); - } - - findTitle() { - cy.get(`h1`).should(`have.text`, `Model registry`); - } -} - -export const home = new Home(); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts new file mode 100644 index 00000000..9133b2ce --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts @@ -0,0 +1,192 @@ +import { appChrome } from '~/__tests__/cypress/cypress/pages/appChrome'; +// import { TableRow } from './components/table'; +// import { Modal } from './components/Modal'; + +// TODO: Uncomment when the modal is implemented +// class LabelModal extends Modal { +// constructor() { +// super('Labels'); +// } + +// findModalSearchInput() { +// return cy.findByTestId('label-modal-search'); +// } + +// findCloseModal() { +// return cy.findByTestId('close-modal'); +// } + +// shouldContainsModalLabels(labels: string[]) { +// cy.findByTestId('modal-label-group').within(() => labels.map((label) => cy.contains(label))); +// return this; +// } +// } + +// TODO: Uncomment when the table is implemented +// class ModelRegistryTableRow extends TableRow { +// findName() { +// return this.find().findByTestId('model-name'); +// } + +// findDescription() { +// return this.find().findByTestId('description'); +// } + +// findOwner() { +// return this.find().findByTestId('registered-model-owner'); +// } + +// findLabelPopoverText() { +// return this.find().findByTestId('popover-label-text'); +// } + +// findLabelModalText() { +// return this.find().findByTestId('modal-label-text'); +// } + +// shouldContainsPopoverLabels(labels: string[]) { +// cy.findByTestId('popover-label-group').within(() => labels.map((label) => cy.contains(label))); +// return this; +// } + +// findModelVersionName() { +// return this.find().findByTestId('model-version-name'); +// } +// } + +class ModelRegistry { + landingPage() { + cy.visit('/'); + this.waitLanding(); + } + + visit() { + cy.visit(`/modelRegistry`); + this.wait(); + } + + navigate() { + appChrome.findNavItem('Model Registry').click(); + this.wait(); + } + + private wait() { + cy.findByTestId('app-page-title').should('exist'); + cy.findByTestId('app-page-title').contains('Model Registry'); + cy.testA11y(); + } + + private waitLanding() { + cy.findByTestId('home-page').should('be.visible'); + } + + shouldBeEmpty() { + cy.findByTestId('empty-state-title').should('exist'); + return this; + } + + findModelRegistryEmptyState() { + return cy.findByTestId('empty-model-registries-state'); + } + + shouldregisteredModelsEmpty() { + cy.findByTestId('empty-registered-models').should('exist'); + } + + shouldmodelVersionsEmpty() { + cy.findByTestId('empty-model-versions').should('exist'); + } + + shouldModelRegistrySelectorExist() { + cy.findByTestId('model-registry-selector-dropdown').should('exist'); + } + + shouldtableToolbarExist() { + cy.findByTestId('registered-models-table-toolbar').should('exist'); + } + + tabEnabled() { + appChrome.findNavItem('Model Registry').should('exist'); + return this; + } + + tabDisabled() { + appChrome.findNavItem('Model Registry').should('not.exist'); + return this; + } + + findTable() { + return cy.findByTestId('registered-model-table'); + } + + findModelVersionsTable() { + return cy.findByTestId('model-versions-table'); + } + + findTableRows() { + return this.findTable().find('tbody tr'); + } + + findModelVersionsTableRows() { + return this.findModelVersionsTable().find('tbody tr'); + } + + // TODO: Uncomment when the table row is implemented + // getRow(name: string) { + // return new ModelRegistryTableRow(() => + // this.findTable().find(`[data-label="Model name"]`).contains(name).parents('tr'), + // ); + // } + + // getModelVersionRow(name: string) { + // return new ModelRegistryTableRow(() => + // this.findModelVersionsTable() + // .find(`[data-label="Version name"]`) + // .contains(name) + // .parents('tr'), + // ); + // } + + findRegisteredModelTableHeaderButton(name: string) { + return this.findTable().find('thead').findByRole('button', { name }); + } + + findModelRegistry() { + return cy.findByTestId('model-registry-selector-dropdown'); + } + + findModelVersionsTableHeaderButton(name: string) { + return this.findModelVersionsTable().find('thead').findByRole('button', { name }); + } + + findTableSearch() { + return cy.findByTestId('registered-model-table-search'); + } + + findModelVersionsTableSearch() { + return cy.findByTestId('model-versions-table-search'); + } + + findModelBreadcrumbItem() { + return cy.findByTestId('breadcrumb-model'); + } + + findModelVersionsTableKebab() { + return cy.findByTestId('model-versions-table-kebab-action'); + } + + findModelVersionsHeaderAction() { + return cy.findByTestId('model-version-action-toggle'); + } + + findModelVersionsTableFilter() { + return cy.findByTestId('model-versions-table-filter'); + } + + findRegisterModelButton() { + return cy.findByRole('button', { name: 'Register model' }); + } +} + +export const modelRegistry = new ModelRegistry(); +// export const labelModal = new LabelModal(); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts new file mode 100644 index 00000000..7b55feae --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/pages/modelRegistryView/registerModelPage.ts @@ -0,0 +1,61 @@ +export enum FormFieldSelector { + MODEL_NAME = '#model-name', + MODEL_DESCRIPTION = '#model-description', + VERSION_NAME = '#version-name', + VERSION_DESCRIPTION = '#version-description', + SOURCE_MODEL_FORMAT = '#source-model-format', + SOURCE_MODEL_FORMAT_VERSION = '#source-model-format-version', + LOCATION_TYPE_OBJECT_STORAGE = '#location-type-object-storage', + LOCATION_ENDPOINT = '#location-endpoint', + LOCATION_BUCKET = '#location-bucket', + LOCATION_REGION = '#location-region', + LOCATION_PATH = '#location-path', + LOCATION_TYPE_URI = '#location-type-uri', + LOCATION_URI = '#location-uri', +} + +class RegisterModelPage { + visit() { + const preferredModelRegistry = 'modelregistry-sample'; + cy.visit(`/modelRegistry/${preferredModelRegistry}/registerModel`); + this.wait(); + } + + private wait() { + const preferredModelRegistry = 'modelregistry-sample'; + cy.findByTestId('app-page-title').should('exist'); + cy.findByTestId('app-page-title').contains('Register model'); + cy.findByText(`Model registry - ${preferredModelRegistry}`).should('exist'); + cy.testA11y(); + } + + findFormField(selector: FormFieldSelector) { + return cy.get(selector); + } + + findObjectStorageAutofillButton() { + return cy.findByTestId('object-storage-autofill-button'); + } + + findConnectionAutofillModal() { + return cy.findByTestId('connection-autofill-modal'); + } + + findProjectSelector() { + return this.findConnectionAutofillModal().findByTestId('project-selector-dropdown'); + } + + findConnectionSelector() { + return this.findConnectionAutofillModal().findByTestId('select-data-connection'); + } + + findAutofillButton() { + return cy.findByTestId('autofill-modal-button'); + } + + findSubmitButton() { + return cy.findByTestId('create-button'); + } +} + +export const registerModelPage = new RegisterModelPage(); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts new file mode 100644 index 00000000..e96daa3c --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/api.ts @@ -0,0 +1,138 @@ +import type { GenericStaticResponse, RouteHandlerController } from 'cypress/types/net-stubbing'; +import type { + ModelArtifact, + ModelArtifactList, + ModelRegistryResponse, + ModelVersion, + ModelVersionList, + RegisteredModel, + RegisteredModelList, +} from '~/app/types'; + +type SuccessErrorResponse = { + success: boolean; + error?: string; +}; + +type ApiResponse = + | V + | GenericStaticResponse + | RouteHandlerController; + +type Replacement = Record; +type Query = Record; + +type Options = { path?: Replacement; query?: Query; times?: number } | null; + +/* eslint-disable @typescript-eslint/no-namespace */ +declare global { + namespace Cypress { + interface Chainable { + interceptApi: (( + type: 'GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models', + options: { path: { modelRegistryName: string; apiVersion: string } }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'POST /api/:apiVersion/model_registry/:modelRegistryName/registered_models', + options: { path: { modelRegistryName: string; apiVersion: string } }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions', + options: { + path: { modelRegistryName: string; apiVersion: string; registeredModelId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'POST /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions', + options: { + path: { modelRegistryName: string; apiVersion: string; registeredModelId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId', + options: { + path: { modelRegistryName: string; apiVersion: string; registeredModelId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'PATCH /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId', + options: { + path: { modelRegistryName: string; apiVersion: string; registeredModelId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId', + options: { + path: { modelRegistryName: string; apiVersion: string; modelVersionId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId/artifacts', + options: { + path: { modelRegistryName: string; apiVersion: string; modelVersionId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'POST /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId/artifacts', + options: { + path: { modelRegistryName: string; apiVersion: string; modelVersionId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'PATCH /api/:apiVersion/model_registry/:modelRegistryName/model_versions/:modelVersionId', + options: { + path: { modelRegistryName: string; apiVersion: string; modelVersionId: number }; + }, + response: ApiResponse, + ) => Cypress.Chainable) & + (( + type: 'GET /api/:apiVersion/model_registry', + options: { path: { apiVersion: string } }, + response: ApiResponse, + ) => Cypress.Chainable); + } + } +} + +Cypress.Commands.add( + 'interceptApi', + (type: string, ...args: [Options | null, ApiResponse] | [ApiResponse]) => { + if (!type) { + throw new Error('Invalid type parameter.'); + } + const options = args.length === 2 ? args[0] : null; + const response = (args.length === 2 ? args[1] : args[0]) ?? ''; + + const pathParts = type.match(/:[a-z][a-zA-Z0-9-_]+/g); + const [method, staticPathname] = type.split(' '); + let pathname = staticPathname; + if (pathParts?.length) { + if (!options || !options.path) { + throw new Error(`${type}: missing path replacements`); + } + const { path: pathReplacements } = options; + pathParts.forEach((p) => { + // remove the starting colun from the regex match + const part = p.substring(1); + const replacement = pathReplacements[part]; + if (!replacement) { + throw new Error(`${type} missing path replacement: ${part}`); + } + pathname = pathname.replace(new RegExp(`:${part}\\b`), replacement); + }); + } + return cy.intercept( + { method, pathname, query: options?.query, ...(options?.times && { times: options.times }) }, + response, + ); + }, +); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts new file mode 100644 index 00000000..98f42c4a --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/application.ts @@ -0,0 +1,271 @@ +import type { MatcherOptions } from '@testing-library/cypress'; +import type { Matcher, MatcherOptions as DTLMatcherOptions } from '@testing-library/dom'; +// import type { UserAuthConfig } from '~/__tests__/cypress/cypress/types'; +// import { TEST_USER } from '~/__tests__/cypress/cypress/utils/e2eUsers'; + +/* eslint-disable @typescript-eslint/no-namespace */ +declare global { + namespace Cypress { + interface Chainable { + // TODO: Uncomment when authorization is enabled + // /** + // * Visits the URL and performs a login if necessary. + // * Uses credentials supplied by environment variables if not provided. + // * + // * @param url the URL to visit + // * @param credentials login credentials + // */ + // visitWithLogin: (url: string, user?: UserAuthConfig) => Cypress.Chainable; + + /** + * Find a patternfly kebab toggle button. + * + * @param isDropdownToggle - True to indicate that it is a dropdown toggle instead of table kebab actions + */ + findKebab: (isDropdownToggle?: boolean) => Cypress.Chainable; + + /** + * Finds a patternfly kebab toggle button, opens the menu, and finds the action. + * + * @param name the name of the action in the kebeb menu + * @param isDropdownToggle - True to indicate that it is a dropdown toggle instead of table kebab actions + */ + findKebabAction: ( + name: string | RegExp, + isDropdownToggle?: boolean, + ) => Cypress.Chainable; + + /** + * Finds a patternfly dropdown item by first opening the dropdown if not already opened. + * + * @param name the name of the item + */ + findDropdownItem: (name: string | RegExp) => Cypress.Chainable; + + /** + * Finds a patternfly dropdown item by data-testid, first opening the dropdown if not already opened. + * + * @param testId the name of the item + */ + findDropdownItemByTestId: (testId: string) => Cypress.Chainable; + /** + * Finds a patternfly select option by first opening the select menu if not already opened. + * + * @param name the name of the option + */ + findSelectOption: (name: string | RegExp) => Cypress.Chainable; + /** + * Finds a patternfly select option by first opening the select menu if not already opened. + * + * @param testId the name of the option + */ + findSelectOptionByTestId: (testId: string) => Cypress.Chainable; + + /** + * Shortcut to first clear the previous value and then type text into DOM element. + * + * @see https://on.cypress.io/type + */ + fill: ( + text: string, + options?: Partial | undefined, + ) => Cypress.Chainable; + + /** + * Returns a PF Switch label for clickable actions. + * + * @param dataId - the data test id you provided to the PF Switch + */ + pfSwitch: (dataId: string) => Cypress.Chainable; + + /** + * Returns a PF Switch input behind the checkbox to compare .should('be.checked') like ops + * + * @param dataId + */ + pfSwitchValue: (dataId: string) => Cypress.Chainable; + + /** + * The bottom two functions, findByTestId and findAllByTestId have the disabled rule + * method-signature-style because they are overwrites. + * Thus, we cannot change it to use the property signature for functions. + * https://typescript-eslint.io/rules/method-signature-style/ + */ + + /** + * Overwrite `findByTestId` to support an array of Matchers. + * When an array of Matches is supplied, parses the data-testid attribute value as a + * whitespace-separated list of words allowing the query to mimic the CSS selector `[data-testid~=value]`. + * + * data-testid="card my-id" + * + * cy.findByTestId(['card', 'my-id']); + * cy.findByTestId('card my-id'); + */ + // eslint-disable-next-line @typescript-eslint/method-signature-style + findByTestId(id: Matcher | Matcher[], options?: MatcherOptions): Chainable; + + /** + * Overwrite `findAllByTestId` to support an array of Matchers. + * When an array of Matches is supplied, parses the data-testid attribute value as a + * whitespace-separated list of words allowing the query to mimic the CSS selector `[data-testid~=value]`. + * + * data-testid="card my-id" + * + * cy.findAllByTestId(['card']); + * cy.findAllByTestId('card my-id'); + */ + // eslint-disable-next-line @typescript-eslint/method-signature-style + findAllByTestId(id: Matcher | Matcher[], options?: MatcherOptions): Chainable; + } + } +} + +// TODO: Uncomment when authorization is enabled +// Cypress.Commands.add('visitWithLogin', (url, user = TEST_USER) => { +// if (Cypress.env('MOCK')) { +// cy.visit(url); +// } else { +// cy.intercept('GET', url, { log: false }).as('visitWithLogin'); + +// cy.visit(url, { failOnStatusCode: false }); + +// cy.wait('@visitWithLogin', { log: false }).then((interception) => { +// if (interception.response?.statusCode === 403) { +// cy.log('Do login'); +// // do login +// cy.get('form[action="/oauth/start"]').submit(); +// cy.findAllByRole('link', user.AUTH_TYPE ? { name: user.AUTH_TYPE } : {}) +// .last() +// .click(); +// cy.get('input[name=username]').type(user.USERNAME); +// cy.get('input[name=password]').type(user.PASSWORD); +// cy.get('form').submit(); +// } else if (interception.response?.statusCode !== 200) { +// throw new Error( +// `Failed to visit '${url}'. Status code: ${ +// interception.response?.statusCode || 'unknown' +// }`, +// ); +// } +// }); +// } +// }); + +Cypress.Commands.add('findKebab', { prevSubject: 'element' }, (subject, isDropdownToggle) => { + Cypress.log({ displayName: 'findKebab' }); + return cy + .wrap(subject) + .findByRole('button', { name: isDropdownToggle ? 'Actions' : 'Kebab toggle' }); +}); + +Cypress.Commands.add( + 'findKebabAction', + { prevSubject: 'element' }, + (subject, name, isDropdownToggle) => { + Cypress.log({ displayName: 'findKebab', message: name }); + return cy + .wrap(subject) + .findKebab(isDropdownToggle) + .then(($el) => { + if ($el.attr('aria-expanded') === 'false') { + cy.wrap($el).click(); + } + return cy.wrap($el.parent()).findByRole('menuitem', { name }); + }); + }, +); + +Cypress.Commands.add('findDropdownItem', { prevSubject: 'element' }, (subject, name) => { + Cypress.log({ displayName: 'findDropdownItem', message: name }); + return cy.wrap(subject).then(($el) => { + if ($el.attr('aria-expanded') === 'false') { + cy.wrap($el).click(); + } + return cy.wrap($el).parent().findByRole('menuitem', { name }); + }); +}); + +Cypress.Commands.add('findDropdownItemByTestId', { prevSubject: 'element' }, (subject, testId) => { + Cypress.log({ displayName: 'findDropdownItemByTestId', message: testId }); + return cy.wrap(subject).then(($el) => { + if ($el.attr('aria-expanded') === 'false') { + cy.wrap($el).click(); + } + return cy.wrap($el).parent().findByTestId(testId); + }); +}); + +Cypress.Commands.add('findSelectOption', { prevSubject: 'element' }, (subject, name) => { + Cypress.log({ displayName: 'findSelectOption', message: name }); + return cy.wrap(subject).then(($el) => { + if ($el.attr('aria-expanded') === 'false') { + cy.wrap($el).click(); + } + //cy.get('[role=listbox]') TODO fix cases where there are multiple listboxes + return cy.findByRole('option', { name }); + }); +}); + +Cypress.Commands.add('findSelectOptionByTestId', { prevSubject: 'element' }, (subject, testId) => { + Cypress.log({ displayName: 'findSelectOptionByTestId', message: testId }); + return cy.wrap(subject).then(($el) => { + if ($el.attr('aria-expanded') === 'false') { + cy.wrap($el).click(); + } + return cy.wrap($el).parent().findByTestId(testId); + }); +}); + +Cypress.Commands.add('fill', { prevSubject: 'optional' }, (subject, text, options) => { + cy.wrap(subject).clear(); + return cy.wrap(subject).type(text, options); +}); + +Cypress.Commands.add('pfSwitch', { prevSubject: 'optional' }, (subject, dataId) => { + Cypress.log({ displayName: 'pfSwitch', message: dataId }); + return cy.wrap(subject).findByTestId(dataId).parent(); +}); + +Cypress.Commands.add('pfSwitchValue', { prevSubject: 'optional' }, (subject, dataId) => { + Cypress.log({ displayName: 'pfSwitchValue', message: dataId }); + return cy.wrap(subject).pfSwitch(dataId).find('[type=checkbox]'); +}); + +Cypress.Commands.overwriteQuery('findByTestId', function findByTestId(...args) { + return enhancedFindByTestId(this, ...args); +}); +Cypress.Commands.overwriteQuery('findAllByTestId', function findAllByTestId(...args) { + return enhancedFindByTestId(this, ...args); +}); + +const enhancedFindByTestId = ( + command: Cypress.Command, + originalFn: Cypress.QueryFn<'findAllByTestId' | 'findByTestId'>, + matcher: Matcher | Matcher[], + options?: MatcherOptions, +) => { + if (Array.isArray(matcher)) { + return originalFn.call( + command, + (content, node) => { + const values = content.trim().split(/\s+/); + return matcher.every((m) => + values.some((v) => { + if (typeof m === 'string' || typeof m === 'number') { + return options && (options as DTLMatcherOptions).exact + ? v.toLowerCase().includes(matcher.toString().toLowerCase()) + : v === String(m); + } + if (typeof m === 'function') { + return m(v, node); + } + return m.test(v); + }), + ); + }, + options, + ); + } + return originalFn.call(command, matcher, options); +}; diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/index.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/index.ts index 4464e031..79ed0c3d 100644 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/index.ts +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/support/commands/index.ts @@ -1,2 +1,4 @@ import '@testing-library/cypress/add-commands'; import './axe'; +import './application'; +import './api'; diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts deleted file mode 100644 index ae7d99d3..00000000 --- a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/application.cy.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { pageNotfound } from '~/__tests__/cypress/cypress/pages/pageNoteFound'; -import { home } from '~/__tests__/cypress/cypress/pages/home'; - -describe('Application', () => { - it('Page not found should render', () => { - pageNotfound.visit(); - }); - - it('Home page should have primary button', () => { - home.visit(); - home.findTitle(); - }); -}); diff --git a/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry.cy.ts b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry.cy.ts new file mode 100644 index 00000000..ac6c0729 --- /dev/null +++ b/clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry.cy.ts @@ -0,0 +1,226 @@ +/* eslint-disable camelcase */ +import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; +import { modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; +import { mockModelRegistryResponse } from '~/__mocks__/mockModelRegistryResponse'; +import type { ModelRegistry, ModelVersion, RegisteredModel } from '~/app/types'; + +const MODEL_REGISTRY_API_VERSION = 'v1'; + +type HandlersProps = { + modelRegistries?: ModelRegistry[]; + registeredModels?: RegisteredModel[]; + modelVersions?: ModelVersion[]; +}; + +const initIntercepts = ({ + modelRegistries = [ + mockModelRegistry({ + name: 'modelregistry-sample', + description: 'New model registry', + displayName: 'Model Registry Sample', + }), + mockModelRegistry({ + name: 'modelregistry-sample-2', + description: 'New model registry 2', + displayName: 'Model Registry Sample 2', + }), + ], + registeredModels = [ + mockRegisteredModel({ + name: 'Fraud detection model', + description: + 'A machine learning model trained to detect fraudulent transactions in financial data', + labels: [ + 'Financial data', + 'Fraud detection', + 'Test label', + 'Machine learning', + 'Next data to be overflow', + ], + }), + mockRegisteredModel({ + name: 'Label modal', + description: + 'A machine learning model trained to detect fraudulent transactions in financial data', + labels: [ + 'Testing label', + 'Financial data', + 'Fraud detection', + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc', + 'Machine learning', + 'Next data to be overflow', + 'Label x', + 'Label y', + 'Label z', + ], + }), + ], + modelVersions = [ + mockModelVersion({ author: 'Author 1' }), + mockModelVersion({ name: 'model version' }), + ], +}: HandlersProps) => { + cy.interceptApi( + `GET /api/:apiVersion/model_registry`, + { + path: { apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockModelRegistryResponse({ model_registry: modelRegistries }), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models`, + { + path: { modelRegistryName: 'modelregistry-sample', apiVersion: MODEL_REGISTRY_API_VERSION }, + }, + mockRegisteredModelList({ items: registeredModels }), + ); + + cy.interceptApi( + `GET /api/:apiVersion/model_registry/:modelRegistryName/registered_models/:registeredModelId/versions`, + { + path: { + modelRegistryName: 'modelregistry-sample', + apiVersion: MODEL_REGISTRY_API_VERSION, + registeredModelId: 1, + }, + }, + mockModelVersionList({ items: modelVersions }), + ); +}; + +describe('Model Registry core', () => { + it('Model Registry Enabled in the cluster', () => { + initIntercepts({}); + + modelRegistry.visit(); + modelRegistry.navigate(); + + modelRegistry.tabEnabled(); + }); + + // it('Renders empty state with no model registries', () => { + // initIntercepts({ + // disableModelRegistryFeature: false, + // modelRegistries: [], + // }); + + // modelRegistry.visit(); + // modelRegistry.navigate(); + // modelRegistry.findModelRegistryEmptyState().should('exist'); + // }); + + it('No registered models in the selected Model Registry', () => { + initIntercepts({ + registeredModels: [], + }); + + modelRegistry.visit(); + modelRegistry.navigate(); + modelRegistry.shouldModelRegistrySelectorExist(); + // modelRegistry.shouldregisteredModelsEmpty(); + }); + + // TODO: Enable when registered model table is enabled + // describe('Registered model table', () => { + // beforeEach(() => { + // initIntercepts({ disableModelRegistryFeature: false }); + // modelRegistry.visit(); + // }); + + // it('Renders row contents', () => { + // const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + // registeredModelRow.findName().contains('Fraud detection model'); + // registeredModelRow + // .findDescription() + // .contains( + // 'A machine learning model trained to detect fraudulent transactions in financial data', + // ); + // registeredModelRow.findOwner().contains('Author 1'); + + // // Label popover + // registeredModelRow.findLabelPopoverText().contains('2 more'); + // registeredModelRow.findLabelPopoverText().click(); + // registeredModelRow.shouldContainsPopoverLabels([ + // 'Machine learning', + // 'Next data to be overflow', + // ]); + // }); + + // it('Renders labels in modal', () => { + // const registeredModelRow2 = modelRegistry.getRow('Label modal'); + // registeredModelRow2.findLabelModalText().contains('6 more'); + // registeredModelRow2.findLabelModalText().click(); + // labelModal.shouldContainsModalLabels([ + // 'Testing label', + // 'Financial', + // 'Financial data', + // 'Fraud detection', + // 'Machine learning', + // 'Next data to be overflow', + // 'Label x', + // 'Label y', + // 'Label z', + // ]); + // labelModal.findModalSearchInput().type('Financial'); + // labelModal.shouldContainsModalLabels(['Financial', 'Financial data']); + // labelModal.findCloseModal().click(); + // }); + + // it('Sort by Model name', () => { + // modelRegistry.findRegisteredModelTableHeaderButton('Model name').click(); + // modelRegistry.findRegisteredModelTableHeaderButton('Model name').should(be.sortAscending); + // modelRegistry.findRegisteredModelTableHeaderButton('Model name').click(); + // modelRegistry.findRegisteredModelTableHeaderButton('Model name').should(be.sortDescending); + // }); + + // it('Sort by Last modified', () => { + // modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortAscending); + // modelRegistry.findRegisteredModelTableHeaderButton('Last modified').click(); + // modelRegistry.findRegisteredModelTableHeaderButton('Last modified').should(be.sortDescending); + // }); + + // it('Filter by keyword', () => { + // modelRegistry.findTableSearch().type('Fraud detection model'); + // modelRegistry.findTableRows().should('have.length', 1); + // modelRegistry.findTableRows().contains('Fraud detection model'); + // }); + // }); +}); + +// TODO: Enable when model registration is there +// describe('Register Model button', () => { +// it('Navigates to register page from empty state', () => { +// initIntercepts({ disableModelRegistryFeature: false, registeredModels: [] }); +// modelRegistry.visit(); +// modelRegistry.findRegisterModelButton().click(); +// cy.findByTestId('app-page-title').should('exist'); +// cy.findByTestId('app-page-title').contains('Register model'); +// cy.findByText('Model registry - modelregistry-sample').should('exist'); +// }); + +// it('Navigates to register page from table toolbar', () => { +// initIntercepts({ disableModelRegistryFeature: false }); +// modelRegistry.visit(); +// modelRegistry.findRegisterModelButton().click(); +// cy.findByTestId('app-page-title').should('exist'); +// cy.findByTestId('app-page-title').contains('Register model'); +// cy.findByText('Model registry - modelregistry-sample').should('exist'); +// }); + +// it('should be accessible for non-admin users', () => { +// asProjectEditUser(); +// initIntercepts({ +// disableModelRegistryFeature: false, +// allowed: false, +// }); + +// modelRegistry.visit(); +// modelRegistry.navigate(); +// modelRegistry.shouldModelRegistrySelectorExist(); +// }); +// }); diff --git a/clients/ui/frontend/src/app/App.tsx b/clients/ui/frontend/src/app/App.tsx index 2a8a7a81..97e644f6 100644 --- a/clients/ui/frontend/src/app/App.tsx +++ b/clients/ui/frontend/src/app/App.tsx @@ -23,6 +23,7 @@ import NavSidebar from './NavSidebar'; import AppRoutes from './AppRoutes'; import { AppContext } from './AppContext'; import { useSettings } from './useSettings'; +import { ModelRegistrySelectorContextProvider } from './context/ModelRegistrySelectorContext'; const App: React.FC = () => { const { @@ -108,7 +109,9 @@ const App: React.FC = () => { isManagedSidebar sidebar={} > - + + + ); diff --git a/clients/ui/frontend/src/app/AppRoutes.tsx b/clients/ui/frontend/src/app/AppRoutes.tsx index 10051e6d..c2383da6 100644 --- a/clients/ui/frontend/src/app/AppRoutes.tsx +++ b/clients/ui/frontend/src/app/AppRoutes.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { Route, Routes } from 'react-router-dom'; +import { Navigate, Route, Routes } from 'react-router-dom'; import { NotFound } from './pages/notFound/NotFound'; import ModelRegistrySettingsRoutes from './pages/settings/ModelRegistrySettingsRoutes'; import ModelRegistryRoutes from './pages/modelRegistry/ModelRegistryRoutes'; @@ -42,7 +42,7 @@ export const useAdminSettings = (): NavDataItem[] => { export const useNavData = (): NavDataItem[] => [ { label: 'Model Registry', - path: '/', + path: '/modelRegistry', }, ...useAdminSettings(), ]; @@ -52,12 +52,13 @@ const AppRoutes: React.FC = () => { return ( - } /> + } /> + } /> } /> { // TODO: Remove the linter skip when we implement authentication // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - isAdmin && } /> + isAdmin && } /> } ); diff --git a/clients/ui/frontend/src/app/api/__tests__/errorUtils.spec.ts b/clients/ui/frontend/src/app/api/__tests__/errorUtils.spec.ts index 3c225152..fdf473f1 100644 --- a/clients/ui/frontend/src/app/api/__tests__/errorUtils.spec.ts +++ b/clients/ui/frontend/src/app/api/__tests__/errorUtils.spec.ts @@ -1,5 +1,5 @@ import { NotReadyError } from '~/utilities/useFetchState'; -import { APIError } from '~/types'; +import { APIError } from '~/app/api/types'; import { handleRestFailures } from '~/app/api/errorUtils'; import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; @@ -12,8 +12,10 @@ describe('handleRestFailures', () => { it('should handle and throw model registry errors', async () => { const statusMock: APIError = { - code: '', - message: 'error', + error: { + code: '', + message: 'error', + }, }; await expect(handleRestFailures(Promise.resolve(statusMock))).rejects.toThrow('error'); diff --git a/clients/ui/frontend/src/app/api/__tests__/service.spec.ts b/clients/ui/frontend/src/app/api/__tests__/service.spec.ts index 1e2a36e2..6bfe6e6a 100644 --- a/clients/ui/frontend/src/app/api/__tests__/service.spec.ts +++ b/clients/ui/frontend/src/app/api/__tests__/service.spec.ts @@ -45,18 +45,21 @@ const K8sAPIOptionsMock = {}; describe('createRegisteredModel', () => { it('should call restCREATE and handleRestFailures to create registered model', () => { expect( - createRegisteredModel('hostPath', 'model-registry-1')(K8sAPIOptionsMock, { - description: 'test', - externalID: '1', - name: 'test new registered model', - state: ModelState.LIVE, - customProperties: {}, - }), + createRegisteredModel(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + K8sAPIOptionsMock, + { + description: 'test', + externalID: '1', + name: 'test new registered model', + state: ModelState.LIVE, + customProperties: {}, + }, + ), ).toBe(mockResultPromise); expect(restCREATEMock).toHaveBeenCalledTimes(1); expect(restCREATEMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/registered_models`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/registered_models`, { description: 'test', externalID: '1', @@ -75,20 +78,23 @@ describe('createRegisteredModel', () => { describe('createModelVersion', () => { it('should call restCREATE and handleRestFailures to create model version', () => { expect( - createModelVersion('hostPath', 'model-registry-1')(K8sAPIOptionsMock, { - description: 'test', - externalID: '1', - author: 'test author', - registeredModelId: '1', - name: 'test new model version', - state: ModelState.LIVE, - customProperties: {}, - }), + createModelVersion(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + K8sAPIOptionsMock, + { + description: 'test', + externalID: '1', + author: 'test author', + registeredModelId: '1', + name: 'test new model version', + state: ModelState.LIVE, + customProperties: {}, + }, + ), ).toBe(mockResultPromise); expect(restCREATEMock).toHaveBeenCalledTimes(1); expect(restCREATEMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_versions`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_versions`, { description: 'test', externalID: '1', @@ -109,7 +115,9 @@ describe('createModelVersion', () => { describe('createModelVersionForRegisteredModel', () => { it('should call restCREATE and handleRestFailures to create model version for a model', () => { expect( - createModelVersionForRegisteredModel('hostPath', 'model-registry-1')(K8sAPIOptionsMock, '1', { + createModelVersionForRegisteredModel( + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + )(K8sAPIOptionsMock, '1', { description: 'test', externalID: '1', author: 'test author', @@ -121,8 +129,8 @@ describe('createModelVersionForRegisteredModel', () => { ).toBe(mockResultPromise); expect(restCREATEMock).toHaveBeenCalledTimes(1); expect(restCREATEMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/registered_models/1/versions`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/registered_models/1/versions`, { description: 'test', externalID: '1', @@ -143,25 +151,28 @@ describe('createModelVersionForRegisteredModel', () => { describe('createModelArtifact', () => { it('should call restCREATE and handleRestFailures to create model artifact', () => { expect( - createModelArtifact('hostPath', 'model-registry-1')(K8sAPIOptionsMock, { - description: 'test', - externalID: 'test', - uri: 'test-uri', - state: ModelArtifactState.LIVE, - name: 'test-name', - modelFormatName: 'test-modelformatname', - storageKey: 'teststoragekey', - storagePath: 'teststoragePath', - modelFormatVersion: 'testmodelFormatVersion', - serviceAccountName: 'testserviceAccountname', - customProperties: {}, - artifactType: 'model-artifact', - }), + createModelArtifact(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + K8sAPIOptionsMock, + { + description: 'test', + externalID: 'test', + uri: 'test-uri', + state: ModelArtifactState.LIVE, + name: 'test-name', + modelFormatName: 'test-modelformatname', + storageKey: 'teststoragekey', + storagePath: 'teststoragePath', + modelFormatVersion: 'testmodelFormatVersion', + serviceAccountName: 'testserviceAccountname', + customProperties: {}, + artifactType: 'model-artifact', + }, + ), ).toBe(mockResultPromise); expect(restCREATEMock).toHaveBeenCalledTimes(1); expect(restCREATEMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_artifacts`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_artifacts`, { description: 'test', externalID: 'test', @@ -187,7 +198,9 @@ describe('createModelArtifact', () => { describe('createModelArtifactForModelVersion', () => { it('should call restCREATE and handleRestFailures to create model artifact for version', () => { expect( - createModelArtifactForModelVersion('hostPath', 'model-registry-1')(K8sAPIOptionsMock, '2', { + createModelArtifactForModelVersion( + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + )(K8sAPIOptionsMock, '2', { description: 'test', externalID: 'test', uri: 'test-uri', @@ -204,8 +217,8 @@ describe('createModelArtifactForModelVersion', () => { ).toBe(mockResultPromise); expect(restCREATEMock).toHaveBeenCalledTimes(1); expect(restCREATEMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_versions/2/artifacts`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_versions/2/artifacts`, { description: 'test', externalID: 'test', @@ -230,13 +243,16 @@ describe('createModelArtifactForModelVersion', () => { describe('getRegisteredModel', () => { it('should call restGET and handleRestFailures to fetch registered model', () => { - expect(getRegisteredModel('hostPath', 'model-registry-1')(K8sAPIOptionsMock, '1')).toBe( - mockResultPromise, - ); + expect( + getRegisteredModel(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + K8sAPIOptionsMock, + '1', + ), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/registered_models/1`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/registered_models/1`, {}, K8sAPIOptionsMock, ); @@ -247,13 +263,16 @@ describe('getRegisteredModel', () => { describe('getModelVersion', () => { it('should call restGET and handleRestFailures to fetch model version', () => { - expect(getModelVersion('hostPath', 'model-registry-1')(K8sAPIOptionsMock, '1')).toBe( - mockResultPromise, - ); + expect( + getModelVersion(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + K8sAPIOptionsMock, + '1', + ), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_versions/1`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_versions/1`, {}, K8sAPIOptionsMock, ); @@ -264,13 +283,16 @@ describe('getModelVersion', () => { describe('getModelArtifact', () => { it('should call restGET and handleRestFailures to fetch model version', () => { - expect(getModelArtifact('hostPath', 'model-registry-1')(K8sAPIOptionsMock, '1')).toBe( - mockResultPromise, - ); + expect( + getModelArtifact(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + K8sAPIOptionsMock, + '1', + ), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_artifacts/1`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_artifacts/1`, {}, K8sAPIOptionsMock, ); @@ -281,11 +303,13 @@ describe('getModelArtifact', () => { describe('getListRegisteredModels', () => { it('should call restGET and handleRestFailures to list registered models', () => { - expect(getListRegisteredModels('hostPath', 'model-registry-1')({})).toBe(mockResultPromise); + expect( + getListRegisteredModels(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)({}), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/registered_models`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/registered_models`, {}, K8sAPIOptionsMock, ); @@ -296,11 +320,13 @@ describe('getListRegisteredModels', () => { describe('getListModelArtifacts', () => { it('should call restGET and handleRestFailures to list models artifacts', () => { - expect(getListModelArtifacts('hostPath', 'model-registry-1')({})).toBe(mockResultPromise); + expect( + getListModelArtifacts(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)({}), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_artifacts`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_artifacts`, {}, K8sAPIOptionsMock, ); @@ -311,11 +337,13 @@ describe('getListModelArtifacts', () => { describe('getListModelVersions', () => { it('should call restGET and handleRestFailures to list models versions', () => { - expect(getListModelVersions('hostPath', 'model-registry-1')({})).toBe(mockResultPromise); + expect( + getListModelVersions(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)({}), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_versions`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_versions`, {}, K8sAPIOptionsMock, ); @@ -326,13 +354,16 @@ describe('getListModelVersions', () => { describe('getModelVersionsByRegisteredModel', () => { it('should call restGET and handleRestFailures to list models versions by registered model', () => { - expect(getModelVersionsByRegisteredModel('hostPath', 'model-registry-1')({}, '1')).toBe( - mockResultPromise, - ); + expect( + getModelVersionsByRegisteredModel(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + {}, + '1', + ), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/registered_models/1/versions`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/registered_models/1/versions`, {}, K8sAPIOptionsMock, ); @@ -343,13 +374,16 @@ describe('getModelVersionsByRegisteredModel', () => { describe('getModelArtifactsByModelVersion', () => { it('should call restGET and handleRestFailures to list models artifacts by model version', () => { - expect(getModelArtifactsByModelVersion('hostPath', 'model-registry-1')({}, '1')).toBe( - mockResultPromise, - ); + expect( + getModelArtifactsByModelVersion(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( + {}, + '1', + ), + ).toBe(mockResultPromise); expect(restGETMock).toHaveBeenCalledTimes(1); expect(restGETMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_versions/1/artifacts`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_versions/1/artifacts`, {}, K8sAPIOptionsMock, ); @@ -361,7 +395,7 @@ describe('getModelArtifactsByModelVersion', () => { describe('patchRegisteredModel', () => { it('should call restPATCH and handleRestFailures to update registered model', () => { expect( - patchRegisteredModel('hostPath', 'model-registry-1')( + patchRegisteredModel(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( K8sAPIOptionsMock, { description: 'new test' }, '1', @@ -369,8 +403,8 @@ describe('patchRegisteredModel', () => { ).toBe(mockResultPromise); expect(restPATCHMock).toHaveBeenCalledTimes(1); expect(restPATCHMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/registered_models/1`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/registered_models/1`, { description: 'new test' }, K8sAPIOptionsMock, ); @@ -382,7 +416,7 @@ describe('patchRegisteredModel', () => { describe('patchModelVersion', () => { it('should call restPATCH and handleRestFailures to update model version', () => { expect( - patchModelVersion('hostPath', 'model-registry-1')( + patchModelVersion(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( K8sAPIOptionsMock, { description: 'new test' }, '1', @@ -390,8 +424,8 @@ describe('patchModelVersion', () => { ).toBe(mockResultPromise); expect(restPATCHMock).toHaveBeenCalledTimes(1); expect(restPATCHMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_versions/1`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_versions/1`, { description: 'new test' }, K8sAPIOptionsMock, ); @@ -403,7 +437,7 @@ describe('patchModelVersion', () => { describe('patchModelArtifact', () => { it('should call restPATCH and handleRestFailures to update model artifact', () => { expect( - patchModelArtifact('hostPath', 'model-registry-1')( + patchModelArtifact(`/api/${BFF_API_VERSION}/model_registry/model-registry-1/`)( K8sAPIOptionsMock, { description: 'new test' }, '1', @@ -411,8 +445,8 @@ describe('patchModelArtifact', () => { ).toBe(mockResultPromise); expect(restPATCHMock).toHaveBeenCalledTimes(1); expect(restPATCHMock).toHaveBeenCalledWith( - 'hostPath', - `/api/${BFF_API_VERSION}/model_registry/model-registry-1/model_artifacts/1`, + `/api/${BFF_API_VERSION}/model_registry/model-registry-1/`, + `/model_artifacts/1`, { description: 'new test' }, K8sAPIOptionsMock, ); diff --git a/clients/ui/frontend/src/app/api/apiUtils.ts b/clients/ui/frontend/src/app/api/apiUtils.ts index d4adff6c..0af63847 100644 --- a/clients/ui/frontend/src/app/api/apiUtils.ts +++ b/clients/ui/frontend/src/app/api/apiUtils.ts @@ -1,5 +1,6 @@ -import { APIOptions } from '~/types'; +import { APIOptions } from '~/app/api/types'; import { EitherOrNone } from '~/typeHelpers'; +import { ModelRegistryResponse } from '~/app/types'; export const mergeRequestInit = ( opts: APIOptions = {}, @@ -161,3 +162,12 @@ export const restDELETE = ( queryParams, parseJSON: options?.parseJSON, }); + +export const isModelRegistryResponse = (response: unknown): response is ModelRegistryResponse => { + if (typeof response === 'object' && response !== null) { + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + const modelRegistryResponse = response as { model_registry?: unknown }; + return Array.isArray(modelRegistryResponse.model_registry); + } + return false; +}; diff --git a/clients/ui/frontend/src/app/api/errorUtils.ts b/clients/ui/frontend/src/app/api/errorUtils.ts index 4cb92823..35fbdc2d 100644 --- a/clients/ui/frontend/src/app/api/errorUtils.ts +++ b/clients/ui/frontend/src/app/api/errorUtils.ts @@ -1,8 +1,7 @@ -import { APIError } from '~/types'; +import { APIError } from '~/app/api/types'; import { isCommonStateError } from '~/utilities/useFetchState'; -const isError = (e: unknown): e is APIError => - typeof e === 'object' && e !== null && ['code', 'message'].every((key) => key in e); +const isError = (e: unknown): e is APIError => typeof e === 'object' && e !== null && 'error' in e; export const handleRestFailures = (promise: Promise): Promise => promise @@ -14,7 +13,7 @@ export const handleRestFailures = (promise: Promise): Promise => }) .catch((e) => { if (isError(e)) { - throw new Error(e.message); + throw new Error(e.error.message); } if (isCommonStateError(e)) { // Common state errors are handled by useFetchState at storage level, let them deal with it diff --git a/clients/ui/frontend/src/app/api/k8s.ts b/clients/ui/frontend/src/app/api/k8s.ts index e17e55db..07f70e98 100644 --- a/clients/ui/frontend/src/app/api/k8s.ts +++ b/clients/ui/frontend/src/app/api/k8s.ts @@ -1,10 +1,17 @@ -import { APIOptions } from '~/types'; +import { APIOptions } from '~/app/api/types'; import { handleRestFailures } from '~/app/api/errorUtils'; -import { restGET } from '~/app/api/apiUtils'; +import { isModelRegistryResponse, restGET } from '~/app/api/apiUtils'; import { ModelRegistry } from '~/app/types'; import { BFF_API_VERSION } from '~/app/const'; -export const getModelRegistries = +export const getListModelRegistries = (hostPath: string) => - (opts: APIOptions): Promise => - handleRestFailures(restGET(hostPath, `/api/${BFF_API_VERSION}/model_registry`, {}, opts)); + (opts: APIOptions): Promise => + handleRestFailures(restGET(hostPath, `/api/${BFF_API_VERSION}/model_registry`, {}, opts)).then( + (response) => { + if (isModelRegistryResponse(response)) { + return response.model_registry; + } + throw new Error('Invalid response format'); + }, + ); diff --git a/clients/ui/frontend/src/app/api/service.ts b/clients/ui/frontend/src/app/api/service.ts index 42f8dbb5..696c46ba 100644 --- a/clients/ui/frontend/src/app/api/service.ts +++ b/clients/ui/frontend/src/app/api/service.ts @@ -10,218 +10,107 @@ import { RegisteredModel, } from '~/app/types'; import { restCREATE, restGET, restPATCH } from '~/app/api/apiUtils'; -import { APIOptions } from '~/types'; +import { APIOptions } from '~/app/api/types'; import { handleRestFailures } from '~/app/api/errorUtils'; -import { BFF_API_VERSION } from '~/app/const'; export const createRegisteredModel = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, data: CreateRegisteredModelData): Promise => - handleRestFailures( - restCREATE( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/registered_models`, - data, - {}, - opts, - ), - ); + handleRestFailures(restCREATE(hostPath, `/registered_models`, data, {}, opts)); export const createModelVersion = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, data: CreateModelVersionData): Promise => - handleRestFailures( - restCREATE( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_versions`, - data, - {}, - opts, - ), - ); + handleRestFailures(restCREATE(hostPath, `/model_versions`, data, {}, opts)); + export const createModelVersionForRegisteredModel = - (hostPath: string, mrName: string) => + (hostPath: string) => ( opts: APIOptions, registeredModelId: string, data: CreateModelVersionData, ): Promise => handleRestFailures( - restCREATE( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/registered_models/${registeredModelId}/versions`, - data, - {}, - opts, - ), + restCREATE(hostPath, `/registered_models/${registeredModelId}/versions`, data, {}, opts), ); export const createModelArtifact = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, data: CreateModelArtifactData): Promise => - handleRestFailures( - restCREATE( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_artifacts`, - data, - {}, - opts, - ), - ); + handleRestFailures(restCREATE(hostPath, `/model_artifacts`, data, {}, opts)); export const createModelArtifactForModelVersion = - (hostPath: string, mrName: string) => + (hostPath: string) => ( opts: APIOptions, modelVersionId: string, data: CreateModelArtifactData, ): Promise => handleRestFailures( - restCREATE( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_versions/${modelVersionId}/artifacts`, - data, - {}, - opts, - ), + restCREATE(hostPath, `/model_versions/${modelVersionId}/artifacts`, data, {}, opts), ); export const getRegisteredModel = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, registeredModelId: string): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/registered_models/${registeredModelId}`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/registered_models/${registeredModelId}`, {}, opts)); export const getModelVersion = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, modelversionId: string): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_versions/${modelversionId}`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/model_versions/${modelversionId}`, {}, opts)); export const getModelArtifact = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, modelArtifactId: string): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_artifacts/${modelArtifactId}`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/model_artifacts/${modelArtifactId}`, {}, opts)); export const getListModelArtifacts = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_artifacts`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/model_artifacts`, {}, opts)); export const getListModelVersions = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_versions`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/model_versions`, {}, opts)); export const getListRegisteredModels = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/registered_models`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/registered_models`, {}, opts)); export const getModelVersionsByRegisteredModel = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, registeredmodelId: string): Promise => handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/registered_models/${registeredmodelId}/versions`, - {}, - opts, - ), + restGET(hostPath, `/registered_models/${registeredmodelId}/versions`, {}, opts), ); export const getModelArtifactsByModelVersion = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, modelVersionId: string): Promise => - handleRestFailures( - restGET( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_versions/${modelVersionId}/artifacts`, - {}, - opts, - ), - ); + handleRestFailures(restGET(hostPath, `/model_versions/${modelVersionId}/artifacts`, {}, opts)); export const patchRegisteredModel = - (hostPath: string, mrName: string) => + (hostPath: string) => ( opts: APIOptions, data: Partial, registeredModelId: string, ): Promise => - handleRestFailures( - restPATCH( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/registered_models/${registeredModelId}`, - data, - opts, - ), - ); + handleRestFailures(restPATCH(hostPath, `/registered_models/${registeredModelId}`, data, opts)); export const patchModelVersion = - (hostPath: string, mrName: string) => + (hostPath: string) => (opts: APIOptions, data: Partial, modelversionId: string): Promise => - handleRestFailures( - restPATCH( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_versions/${modelversionId}`, - data, - opts, - ), - ); + handleRestFailures(restPATCH(hostPath, `/model_versions/${modelversionId}`, data, opts)); export const patchModelArtifact = - (hostPath: string, mrName: string) => + (hostPath: string) => ( opts: APIOptions, data: Partial, modelartifactId: string, ): Promise => - handleRestFailures( - restPATCH( - hostPath, - `/api/${BFF_API_VERSION}/model_registry/${mrName}/model_artifacts/${modelartifactId}`, - data, - opts, - ), - ); + handleRestFailures(restPATCH(hostPath, `/model_artifacts/${modelartifactId}`, data, opts)); diff --git a/clients/ui/frontend/src/app/api/types.ts b/clients/ui/frontend/src/app/api/types.ts new file mode 100644 index 00000000..e7335512 --- /dev/null +++ b/clients/ui/frontend/src/app/api/types.ts @@ -0,0 +1,19 @@ +export type APIOptions = { + dryRun?: boolean; + signal?: AbortSignal; + parseJSON?: boolean; +}; + +export type APIError = { + error: { + code: string; + message: string; + }; +}; + +export type APIState = { + /** If API will successfully call */ + apiAvailable: boolean; + /** The available API functions */ + api: T; +}; diff --git a/clients/ui/frontend/src/app/api/useAPIState.ts b/clients/ui/frontend/src/app/api/useAPIState.ts new file mode 100644 index 00000000..4783e8cb --- /dev/null +++ b/clients/ui/frontend/src/app/api/useAPIState.ts @@ -0,0 +1,32 @@ +import * as React from 'react'; +import { APIState } from '~/app/api/types'; + +const useAPIState = ( + hostPath: string | null, + createAPI: (path: string) => T, +): [apiState: APIState, refreshAPIState: () => void] => { + const [internalAPIToggleState, setInternalAPIToggleState] = React.useState(false); + + const refreshAPIState = React.useCallback(() => { + setInternalAPIToggleState((v) => !v); + }, []); + + const apiState = React.useMemo>(() => { + let path = hostPath; + if (!path) { + // TODO: we need to figure out maybe a stopgap or something + path = ''; + } + const api = createAPI(path); + + return { + apiAvailable: !!path, + api, + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [createAPI, hostPath, internalAPIToggleState]); + + return [apiState, refreshAPIState]; +}; + +export default useAPIState; diff --git a/clients/ui/frontend/src/app/components/EmptyStateErrorMessage.tsx b/clients/ui/frontend/src/app/components/EmptyStateErrorMessage.tsx new file mode 100644 index 00000000..af24ffaa --- /dev/null +++ b/clients/ui/frontend/src/app/components/EmptyStateErrorMessage.tsx @@ -0,0 +1,34 @@ +import * as React from 'react'; +import { + EmptyState, + EmptyStateBody, + Stack, + StackItem, + EmptyStateFooter, +} from '@patternfly/react-core'; +import { PathMissingIcon } from '@patternfly/react-icons'; + +type EmptyStateErrorMessageProps = { + children?: React.ReactNode; + title: string; + bodyText: string; +}; + +const EmptyStateErrorMessage: React.FC = ({ + title, + bodyText, + children, +}) => ( + + + + + {bodyText} + + {children && {children}} + + + +); + +export default EmptyStateErrorMessage; diff --git a/clients/ui/frontend/src/app/context/ModelRegistryContext.tsx b/clients/ui/frontend/src/app/context/ModelRegistryContext.tsx new file mode 100644 index 00000000..6c107e27 --- /dev/null +++ b/clients/ui/frontend/src/app/context/ModelRegistryContext.tsx @@ -0,0 +1,44 @@ +import * as React from 'react'; +import { BFF_API_VERSION } from '~/app/const'; +import useModelRegistryAPIState, { ModelRegistryAPIState } from './useModelRegistryAPIState'; + +export type ModelRegistryContextType = { + apiState: ModelRegistryAPIState; + refreshAPIState: () => void; +}; + +type ModelRegistryContextProviderProps = { + children: React.ReactNode; + modelRegistryName: string; +}; + +export const ModelRegistryContext = React.createContext({ + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + apiState: { apiAvailable: false, api: null as unknown as ModelRegistryAPIState['api'] }, + refreshAPIState: () => undefined, +}); + +export const ModelRegistryContextProvider: React.FC = ({ + children, + modelRegistryName, +}) => { + const hostPath = modelRegistryName + ? `/api/${BFF_API_VERSION}/model_registry/${modelRegistryName}` + : null; + + const [apiState, refreshAPIState] = useModelRegistryAPIState(hostPath); + + return ( + ({ + apiState, + refreshAPIState, + }), + [apiState, refreshAPIState], + )} + > + {children} + + ); +}; diff --git a/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx b/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx new file mode 100644 index 00000000..273c900a --- /dev/null +++ b/clients/ui/frontend/src/app/context/ModelRegistrySelectorContext.tsx @@ -0,0 +1,58 @@ +import * as React from 'react'; +import { ModelRegistry } from '~/app/types'; +import useModelRegistries from '~/app/hooks/useModelRegistries'; + +export type ModelRegistrySelectorContextType = { + modelRegistriesLoaded: boolean; + modelRegistriesLoadError?: Error; + modelRegistries: ModelRegistry[]; + preferredModelRegistry: ModelRegistry | undefined; + updatePreferredModelRegistry: (modelRegistry: ModelRegistry | undefined) => void; +}; + +type ModelRegistrySelectorContextProviderProps = { + children: React.ReactNode; +}; + +export const ModelRegistrySelectorContext = React.createContext({ + modelRegistriesLoaded: false, + modelRegistriesLoadError: undefined, + modelRegistries: [], + preferredModelRegistry: undefined, + updatePreferredModelRegistry: () => undefined, +}); + +export const ModelRegistrySelectorContextProvider: React.FC< + ModelRegistrySelectorContextProviderProps +> = ({ children, ...props }) => ( + + {children} + +); + +const EnabledModelRegistrySelectorContextProvider: React.FC< + ModelRegistrySelectorContextProviderProps +> = ({ children }) => { + const [modelRegistries, isLoaded, error] = useModelRegistries(); + const [preferredModelRegistry, setPreferredModelRegistry] = + React.useState(undefined); + + const firstModelRegistry = modelRegistries.length > 0 ? modelRegistries[0] : null; + + const contextValue = React.useMemo( + () => ({ + modelRegistriesLoaded: isLoaded, + modelRegistriesLoadError: error, + modelRegistries, + preferredModelRegistry: preferredModelRegistry ?? firstModelRegistry ?? undefined, + updatePreferredModelRegistry: setPreferredModelRegistry, + }), + [isLoaded, error, modelRegistries, preferredModelRegistry, firstModelRegistry], + ); + + return ( + + {children} + + ); +}; diff --git a/clients/ui/frontend/src/app/context/useModelRegistryAPIState.tsx b/clients/ui/frontend/src/app/context/useModelRegistryAPIState.tsx new file mode 100644 index 00000000..9b1465ba --- /dev/null +++ b/clients/ui/frontend/src/app/context/useModelRegistryAPIState.tsx @@ -0,0 +1,54 @@ +import React from 'react'; +import { APIState } from '~/app/api/types'; +import { ModelRegistryAPIs } from '~/app/types'; +import { + createModelArtifact, + createModelArtifactForModelVersion, + createModelVersion, + createModelVersionForRegisteredModel, + createRegisteredModel, + getListModelArtifacts, + getListModelVersions, + getListRegisteredModels, + getModelArtifact, + getModelArtifactsByModelVersion, + getModelVersion, + getModelVersionsByRegisteredModel, + getRegisteredModel, + patchModelArtifact, + patchModelVersion, + patchRegisteredModel, +} from '~/app/api/service'; +import useAPIState from '~/app/api/useAPIState'; + +export type ModelRegistryAPIState = APIState; + +const useModelRegistryAPIState = ( + hostPath: string | null, +): [apiState: ModelRegistryAPIState, refreshAPIState: () => void] => { + const createAPI = React.useCallback( + (path: string) => ({ + createRegisteredModel: createRegisteredModel(path), + createModelVersion: createModelVersion(path), + createModelVersionForRegisteredModel: createModelVersionForRegisteredModel(path), + createModelArtifact: createModelArtifact(path), + createModelArtifactForModelVersion: createModelArtifactForModelVersion(path), + getRegisteredModel: getRegisteredModel(path), + getModelVersion: getModelVersion(path), + getModelArtifact: getModelArtifact(path), + listModelArtifacts: getListModelArtifacts(path), + listModelVersions: getListModelVersions(path), + listRegisteredModels: getListRegisteredModels(path), + getModelVersionsByRegisteredModel: getModelVersionsByRegisteredModel(path), + getModelArtifactsByModelVersion: getModelArtifactsByModelVersion(path), + patchRegisteredModel: patchRegisteredModel(path), + patchModelVersion: patchModelVersion(path), + patchModelArtifact: patchModelArtifact(path), + }), + [], + ); + + return useAPIState(hostPath, createAPI); +}; + +export default useModelRegistryAPIState; diff --git a/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts b/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts new file mode 100644 index 00000000..aefe8267 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/__tests__/useModelArtifactsByVersionId.spec.ts @@ -0,0 +1,89 @@ +import { waitFor } from '@testing-library/react'; +import useModelArtifactsByVersionId from '~/app/hooks/useModelArtifactsByVersionId'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; +import { ModelRegistryAPIs } from '~/app/types'; +import { mockModelArtifact } from '~/__mocks__/mockModelArtifact'; +import { testHook } from '~/__tests__/unit/testUtils/hooks'; + +global.fetch = jest.fn(); +// Mock the useModelRegistryAPI hook +jest.mock('~/app/hooks/useModelRegistryAPI', () => ({ + useModelRegistryAPI: jest.fn(), +})); + +const mockUseModelRegistryAPI = jest.mocked(useModelRegistryAPI); + +const mockModelRegistryAPIs: ModelRegistryAPIs = { + createRegisteredModel: jest.fn(), + createModelVersionForRegisteredModel: jest.fn(), + createModelArtifactForModelVersion: jest.fn(), + getRegisteredModel: jest.fn(), + getModelVersion: jest.fn(), + listRegisteredModels: jest.fn(), + getModelVersionsByRegisteredModel: jest.fn(), + getModelArtifactsByModelVersion: jest.fn(), + patchRegisteredModel: jest.fn(), + patchModelVersion: jest.fn(), +}; + +describe('useModelArtifactsByVersionId', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return NotReadyError if API is not available', async () => { + mockUseModelRegistryAPI.mockReturnValue({ + api: mockModelRegistryAPIs, + apiAvailable: false, + refreshAllAPI: jest.fn(), + }); + + const { result } = testHook(useModelArtifactsByVersionId)('version-id'); + + await waitFor(() => { + const [, , error] = result.current; + expect(error?.message).toBe('API not yet available'); + expect(error).toBeInstanceOf(Error); + }); + }); + + it('should return NotReadyError if modelVersionId is not provided', async () => { + mockUseModelRegistryAPI.mockReturnValue({ + api: mockModelRegistryAPIs, + apiAvailable: true, + refreshAllAPI: jest.fn(), + }); + + const { result } = testHook(useModelArtifactsByVersionId)(); + + await waitFor(() => { + const [, , error] = result.current; + expect(error?.message).toBe('No model registeredModel id'); + expect(error).toBeInstanceOf(Error); + }); + }); + + it('should fetch model artifacts if API is available and modelVersionId is provided', async () => { + const mockedResponse = { + items: [mockModelArtifact({ id: 'artifact-1' })], + size: 1, + pageSize: 1, + }; + + mockUseModelRegistryAPI.mockReturnValue({ + api: { + ...mockModelRegistryAPIs, + getModelArtifactsByModelVersion: jest.fn().mockResolvedValue(mockedResponse), + }, + apiAvailable: true, + refreshAllAPI: jest.fn(), + }); + + const { result } = testHook(useModelArtifactsByVersionId)('version-id'); + + await waitFor(() => { + const [data] = result.current; + expect(data).toEqual(mockedResponse); + }); + }); +}); diff --git a/clients/ui/frontend/src/app/hooks/useModelArtifactsByVersionId.ts b/clients/ui/frontend/src/app/hooks/useModelArtifactsByVersionId.ts new file mode 100644 index 00000000..5fb90a17 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useModelArtifactsByVersionId.ts @@ -0,0 +1,27 @@ +import * as React from 'react'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { ModelArtifactList } from '~/app/types'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; + +const useModelArtifactsByVersionId = (modelVersionId?: string): FetchState => { + const { api, apiAvailable } = useModelRegistryAPI(); + const callback = React.useCallback>( + (opts) => { + if (!apiAvailable) { + return Promise.reject(new Error('API not yet available')); + } + if (!modelVersionId) { + return Promise.reject(new Error('No model registeredModel id')); + } + return api.getModelArtifactsByModelVersion(opts, modelVersionId); + }, + [api, apiAvailable, modelVersionId], + ); + return useFetchState( + callback, + { items: [], size: 0, pageSize: 0, nextPageToken: '' }, + { initialPromisePurity: true }, + ); +}; + +export default useModelArtifactsByVersionId; diff --git a/clients/ui/frontend/src/app/hooks/useModelRegistries.ts b/clients/ui/frontend/src/app/hooks/useModelRegistries.ts new file mode 100644 index 00000000..705256a8 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useModelRegistries.ts @@ -0,0 +1,15 @@ +import * as React from 'react'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { ModelRegistry } from '~/app/types'; +import { getListModelRegistries } from '~/app/api/k8s'; + +const useModelRegistries = (): FetchState => { + const listModelRegistries = React.useMemo(() => getListModelRegistries(''), []); + const callback = React.useCallback>( + (opts) => listModelRegistries(opts), + [listModelRegistries], + ); + return useFetchState(callback, [], { initialPromisePurity: true }); +}; + +export default useModelRegistries; diff --git a/clients/ui/frontend/src/app/hooks/useModelRegistryAPI.ts b/clients/ui/frontend/src/app/hooks/useModelRegistryAPI.ts new file mode 100644 index 00000000..5a211568 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useModelRegistryAPI.ts @@ -0,0 +1,16 @@ +import * as React from 'react'; +import { ModelRegistryAPIState } from '~/app/context/useModelRegistryAPIState'; +import { ModelRegistryContext } from '~/app/context/ModelRegistryContext'; + +type UseModelRegistryAPI = ModelRegistryAPIState & { + refreshAllAPI: () => void; +}; + +export const useModelRegistryAPI = (): UseModelRegistryAPI => { + const { apiState, refreshAPIState: refreshAllAPI } = React.useContext(ModelRegistryContext); + + return { + refreshAllAPI, + ...apiState, + }; +}; diff --git a/clients/ui/frontend/src/app/hooks/useModelVersionById.ts b/clients/ui/frontend/src/app/hooks/useModelVersionById.ts new file mode 100644 index 00000000..19b7ecd9 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useModelVersionById.ts @@ -0,0 +1,26 @@ +import * as React from 'react'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { ModelVersion } from '~/app/types'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; + +const useModelVersionById = (modelVersionId?: string): FetchState => { + const { api, apiAvailable } = useModelRegistryAPI(); + + const call = React.useCallback>( + (opts) => { + if (!apiAvailable) { + return Promise.reject(new Error('API not yet available')); + } + if (!modelVersionId) { + return Promise.reject(new Error('No model version id')); + } + + return api.getModelVersion(opts, modelVersionId); + }, + [api, apiAvailable, modelVersionId], + ); + + return useFetchState(call, null); +}; + +export default useModelVersionById; diff --git a/clients/ui/frontend/src/app/hooks/useModelVersionsByRegisteredModel.ts b/clients/ui/frontend/src/app/hooks/useModelVersionsByRegisteredModel.ts new file mode 100644 index 00000000..c8f82f9e --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useModelVersionsByRegisteredModel.ts @@ -0,0 +1,32 @@ +import * as React from 'react'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { ModelVersionList } from '~/app/types'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; + +const useModelVersionsByRegisteredModel = ( + registeredModelId?: string, +): FetchState => { + const { api, apiAvailable } = useModelRegistryAPI(); + + const call = React.useCallback>( + (opts) => { + if (!apiAvailable) { + return Promise.reject(new Error('API not yet available')); + } + if (!registeredModelId) { + return Promise.reject(new Error('No model registeredModel id')); + } + + return api.getModelVersionsByRegisteredModel(opts, registeredModelId); + }, + [api, apiAvailable, registeredModelId], + ); + + return useFetchState( + call, + { items: [], size: 0, pageSize: 0, nextPageToken: '' }, + { initialPromisePurity: true }, + ); +}; + +export default useModelVersionsByRegisteredModel; diff --git a/clients/ui/frontend/src/app/hooks/useRegisteredModelById.ts b/clients/ui/frontend/src/app/hooks/useRegisteredModelById.ts new file mode 100644 index 00000000..c2d45bc8 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useRegisteredModelById.ts @@ -0,0 +1,26 @@ +import * as React from 'react'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { RegisteredModel } from '~/app/types'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; + +const useRegisteredModelById = (registeredModel?: string): FetchState => { + const { api, apiAvailable } = useModelRegistryAPI(); + + const call = React.useCallback>( + (opts) => { + if (!apiAvailable) { + return Promise.reject(new Error('API not yet available')); + } + if (!registeredModel) { + return Promise.reject(new Error('No registered model id')); + } + + return api.getRegisteredModel(opts, registeredModel); + }, + [api, apiAvailable, registeredModel], + ); + + return useFetchState(call, null); +}; + +export default useRegisteredModelById; diff --git a/clients/ui/frontend/src/app/hooks/useRegisteredModels.ts b/clients/ui/frontend/src/app/hooks/useRegisteredModels.ts new file mode 100644 index 00000000..6553c7ae --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/useRegisteredModels.ts @@ -0,0 +1,24 @@ +import * as React from 'react'; +import useFetchState, { FetchState, FetchStateCallbackPromise } from '~/utilities/useFetchState'; +import { RegisteredModelList } from '~/app/types'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; + +const useRegisteredModels = (): FetchState => { + const { api, apiAvailable } = useModelRegistryAPI(); + const callback = React.useCallback>( + (opts) => { + if (!apiAvailable) { + return Promise.reject(new Error('API not yet available')); + } + return api.listRegisteredModels(opts); + }, + [api, apiAvailable], + ); + return useFetchState( + callback, + { items: [], size: 0, pageSize: 0, nextPageToken: '' }, + { initialPromisePurity: true }, + ); +}; + +export default useRegisteredModels; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx new file mode 100644 index 00000000..0c4ec86f --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryCoreLoader.tsx @@ -0,0 +1,137 @@ +import * as React from 'react'; +import { Navigate, Outlet, useParams } from 'react-router-dom'; +import { Bullseye, Alert, Popover, List, ListItem, Button } from '@patternfly/react-core'; +import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; +import ApplicationsPage from '~/app/components/ApplicationsPage'; +import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; +import { ProjectObjectType, typedEmptyImage } from '~/app/components/design/utils'; +import { ModelRegistryContextProvider } from '~/app/context/ModelRegistryContext'; +import TitleWithIcon from '~/app/components/design/TitleWithIcon'; +import EmptyModelRegistryState from './screens/components/EmptyModelRegistryState'; +import InvalidModelRegistry from './screens/InvalidModelRegistry'; +import ModelRegistrySelectorNavigator from './screens/ModelRegistrySelectorNavigator'; +import { modelRegistryUrl } from './screens/routeUtils'; + +type ApplicationPageProps = React.ComponentProps; + +type ModelRegistryCoreLoaderProps = { + getInvalidRedirectPath: (modelRegistry: string) => string; +}; + +type ApplicationPageRenderState = Pick< + ApplicationPageProps, + 'emptyStatePage' | 'empty' | 'headerContent' +>; + +const ModelRegistryCoreLoader: React.FC = ({ + getInvalidRedirectPath, +}) => { + const { modelRegistry } = useParams<{ modelRegistry: string }>(); + + const { + modelRegistriesLoaded, + modelRegistriesLoadError, + modelRegistries, + preferredModelRegistry, + updatePreferredModelRegistry, + } = React.useContext(ModelRegistrySelectorContext); + + const modelRegistryFromRoute = modelRegistries.find((mr) => mr.name === modelRegistry); + + React.useEffect(() => { + if (modelRegistryFromRoute && preferredModelRegistry?.name !== modelRegistryFromRoute.name) { + updatePreferredModelRegistry(modelRegistryFromRoute); + } + }, [modelRegistryFromRoute, updatePreferredModelRegistry, preferredModelRegistry?.name]); + + if (modelRegistriesLoadError) { + return ( + + + {modelRegistriesLoadError.message} + + + ); + } + if (!modelRegistriesLoaded) { + return Loading model registries...; + } + + let renderStateProps: ApplicationPageRenderState & { children?: React.ReactNode }; + if (modelRegistries.length === 0) { + renderStateProps = { + empty: true, + emptyStatePage: ( + ( + + )} + customAction={ + + + The person who gave you your username, or who helped you to log in for the first + time + + Someone in your IT department or help desk + A project manager or developer + + } + > + + + } + /> + ), + headerContent: null, + }; + } else if (modelRegistry) { + const foundModelRegistry = modelRegistries.find((mr) => mr.name === modelRegistry); + if (foundModelRegistry) { + // Render the content + return ( + + + + ); + } + + // They ended up on a non-valid project path + renderStateProps = { + empty: true, + emptyStatePage: , + }; + } else { + // Redirect the namespace suffix into the URL + const redirectModelRegistry = preferredModelRegistry ?? modelRegistries[0]; + return ; + } + + return ( + + } + description="Select a model registry to view and manage your registered models. Model registries provide a structured and organized way to store, share, version, deploy, and track models." + headerContent={ + modelRegistryUrl(modelRegistryName)} + /> + } + {...renderStateProps} + loaded + provideChildrenPadding + /> + ); +}; + +export default ModelRegistryCoreLoader; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx index 1d3e4c0c..40050b5a 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistryRoutes.tsx @@ -1,10 +1,21 @@ import * as React from 'react'; import { Route, Routes } from 'react-router-dom'; -import ModelRegistry from './ModelRegistry'; +import ModelRegistry from './screens/ModelRegistry'; +import ModelRegistryCoreLoader from './ModelRegistryCoreLoader'; +import { modelRegistryUrl } from './screens/routeUtils'; const ModelRegistryRoutes: React.FC = () => ( - } /> + modelRegistryUrl(modelRegistry)} + /> + } + > + } /> + ); diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/InvalidModelRegistry.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/InvalidModelRegistry.tsx new file mode 100644 index 00000000..c1559a72 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/InvalidModelRegistry.tsx @@ -0,0 +1,25 @@ +import * as React from 'react'; +import EmptyStateErrorMessage from '~/app/components/EmptyStateErrorMessage'; +import { modelRegistryUrl } from './routeUtils'; +import ModelRegistrySelectorNavigator from './ModelRegistrySelectorNavigator'; + +type InvalidModelRegistryProps = { + title?: string; + modelRegistry?: string; +}; + +const InvalidModelRegistry: React.FC = ({ title, modelRegistry }) => ( + + modelRegistryUrl(modelRegistryName)} + primary + /> + +); + +export default InvalidModelRegistry; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistry.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx similarity index 63% rename from clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistry.tsx rename to clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx index a9edd66c..d37dda7a 100644 --- a/clients/ui/frontend/src/app/pages/modelRegistry/ModelRegistry.tsx +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistry.tsx @@ -2,6 +2,9 @@ import React from 'react'; import ApplicationsPage from '~/app/components/ApplicationsPage'; import TitleWithIcon from '~/app/components/design/TitleWithIcon'; import { ProjectObjectType } from '~/app/components/design/utils'; +import useRegisteredModels from '~/app/hooks/useRegisteredModels'; +import ModelRegistrySelectorNavigator from './ModelRegistrySelectorNavigator'; +import { modelRegistryUrl } from './routeUtils'; type ModelRegistryProps = Omit< React.ComponentProps, @@ -15,20 +18,27 @@ type ModelRegistryProps = Omit< >; const ModelRegistry: React.FC = ({ ...pageProps }) => { - const [loaded, loadError] = [true, undefined]; // TODO: change with real usage + const [, loaded, loadError] = useRegisteredModels(); return ( + } description="Select a model registry to view and manage your registered models. Model registries provide a structured and organized way to store, share, version, deploy, and track models." + headerContent={ + modelRegistryUrl(modelRegistryName)} + /> + } loadError={loadError} loaded={loaded} provideChildrenPadding removeChildrenTopPadding - /> + > + TODO: Add table of registered models; + ); }; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx new file mode 100644 index 00000000..757eeed4 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelector.tsx @@ -0,0 +1,196 @@ +import * as React from 'react'; +import { + Bullseye, + Button, + DescriptionList, + DescriptionListDescription, + DescriptionListGroup, + DescriptionListTerm, + Divider, + Flex, + FlexItem, + Icon, + MenuToggle, + Popover, + Select, + SelectGroup, + SelectList, + SelectOption, + Tooltip, +} from '@patternfly/react-core'; +import truncateStyles from '@patternfly/react-styles/css/components/Truncate/truncate'; +import { InfoCircleIcon, BlueprintIcon } from '@patternfly/react-icons'; +import { useBrowserStorage } from '~/components/browserStorage'; +import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext'; +import { ModelRegistry } from '~/app/types'; + +const MODEL_REGISTRY_FAVORITE_STORAGE_KEY = 'kubeflow.dashboard.model.registry.favorite'; + +type ModelRegistrySelectorProps = { + modelRegistry: string; + onSelection: (modelRegistry: string) => void; + primary?: boolean; +}; + +const ModelRegistrySelector: React.FC = ({ + modelRegistry, + onSelection, + primary, +}) => { + const { modelRegistries, updatePreferredModelRegistry } = React.useContext( + ModelRegistrySelectorContext, + ); + + const selection = modelRegistries.find((mr) => mr.name === modelRegistry); + const [isOpen, setIsOpen] = React.useState(false); + const [favorites, setFavorites] = useBrowserStorage( + MODEL_REGISTRY_FAVORITE_STORAGE_KEY, + [], + ); + + const selectionDisplayName = selection ? selection.displayName : modelRegistry; + + const toggleLabel = modelRegistries.length === 0 ? 'No model registries' : selectionDisplayName; + + const getMRSelectDescription = (mr: ModelRegistry) => { + const desc = mr.description || mr.name; + if (!desc) { + return; + } + const tooltipContent = ( + + + {`${mr.displayName} description`} + {desc} + + + ); + return ( + + + {desc} + + + ); + }; + + const options = [ + + + {modelRegistries.map((mr) => ( + + {mr.displayName} + + ))} + + , + ]; + + const createFavorites = (favIds: string[]) => { + const favorite: JSX.Element[] = []; + + options.forEach((item) => { + if (item.type === SelectList) { + item.props.children.filter( + (child: JSX.Element) => favIds.includes(child.props.value) && favorite.push(child), + ); + } else if (item.type === SelectGroup) { + item.props.children.props.children.filter( + (child: JSX.Element) => favIds.includes(child.props.value) && favorite.push(child), + ); + } else if (favIds.includes(item.props.value)) { + favorite.push(item); + } + }); + + return favorite; + }; + + const selector = ( + + ); + + if (primary) { + return selector; + } + + return ( + + + + + + + Model registry + + {selector} + {selection && selection.description && ( + + + + + + )} + + + ); +}; + +export default ModelRegistrySelector; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx new file mode 100644 index 00000000..7c606fd3 --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelRegistrySelectorNavigator.tsx @@ -0,0 +1,27 @@ +import * as React from 'react'; +import { useNavigate, useParams } from 'react-router-dom'; +import ModelRegistrySelector from './ModelRegistrySelector'; + +type ModelRegistrySelectorNavigatorProps = { + getRedirectPath: (namespace: string) => string; +} & Omit, 'onSelection' | 'modelRegistry'>; + +const ModelRegistrySelectorNavigator: React.FC = ({ + getRedirectPath, + ...modelRegistrySelectorProps +}) => { + const navigate = useNavigate(); + const { modelRegistry } = useParams<{ modelRegistry: string }>(); + + return ( + { + navigate(getRedirectPath(modelRegistryName)); + }} + modelRegistry={modelRegistry ?? ''} + /> + ); +}; + +export default ModelRegistrySelectorNavigator; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx new file mode 100644 index 00000000..e15ce55e --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/components/EmptyModelRegistryState.tsx @@ -0,0 +1,73 @@ +import React from 'react'; +import { + Button, + ButtonVariant, + EmptyState, + EmptyStateActions, + EmptyStateBody, + EmptyStateFooter, + EmptyStateVariant, +} from '@patternfly/react-core'; +import { PlusCircleIcon } from '@patternfly/react-icons'; + +type EmptyModelRegistryStateType = { + testid?: string; + title: string; + description: string; + primaryActionText?: string; + primaryActionOnClick?: () => void; + secondaryActionText?: string; + secondaryActionOnClick?: () => void; + headerIcon?: React.ComponentType; + customAction?: React.ReactNode; +}; + +const EmptyModelRegistryState: React.FC = ({ + testid, + title, + description, + primaryActionText, + secondaryActionText, + primaryActionOnClick, + secondaryActionOnClick, + headerIcon, + customAction, +}) => ( + + {description} + + {primaryActionText && ( + + + + )} + + {secondaryActionText && ( + + + + )} + + {customAction && {customAction}} + + +); + +export default EmptyModelRegistryState; diff --git a/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts b/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts new file mode 100644 index 00000000..e7ec95ef --- /dev/null +++ b/clients/ui/frontend/src/app/pages/modelRegistry/screens/routeUtils.ts @@ -0,0 +1,51 @@ +export const modelRegistryUrl = (preferredModelRegistry?: string): string => + `/modelRegistry/${preferredModelRegistry}`; + +export const registeredModelsUrl = (preferredModelRegistry?: string): string => + `${modelRegistryUrl(preferredModelRegistry)}/registeredModels`; + +export const registeredModelUrl = (rmId?: string, preferredModelRegistry?: string): string => + `${registeredModelsUrl(preferredModelRegistry)}/${rmId}`; + +export const registeredModelArchiveUrl = (preferredModelRegistry?: string): string => + `${registeredModelsUrl(preferredModelRegistry)}/archive`; + +export const registeredModelArchiveDetailsUrl = ( + rmId?: string, + preferredModelRegistry?: string, +): string => `${registeredModelArchiveUrl(preferredModelRegistry)}/${rmId}`; + +export const modelVersionListUrl = (rmId?: string, preferredModelRegistry?: string): string => + `${registeredModelUrl(rmId, preferredModelRegistry)}/versions`; + +export const modelVersionUrl = ( + mvId: string, + rmId?: string, + preferredModelRegistry?: string, +): string => `${modelVersionListUrl(rmId, preferredModelRegistry)}/${mvId}`; + +export const modelVersionArchiveUrl = (rmId?: string, preferredModelRegistry?: string): string => + `${modelVersionListUrl(rmId, preferredModelRegistry)}/archive`; + +export const modelVersionArchiveDetailsUrl = ( + mvId: string, + rmId?: string, + preferredModelRegistry?: string, +): string => `${modelVersionArchiveUrl(rmId, preferredModelRegistry)}/${mvId}`; + +export const registerModelUrl = (preferredModelRegistry?: string): string => + `${modelRegistryUrl(preferredModelRegistry)}/registerModel`; + +export const registerVersionUrl = (preferredModelRegistry?: string): string => + `${modelRegistryUrl(preferredModelRegistry)}/registerVersion`; + +export const registerVersionForModelUrl = ( + rmId?: string, + preferredModelRegistry?: string, +): string => `${registeredModelUrl(rmId, preferredModelRegistry)}/registerVersion`; + +export const modelVersionDeploymentsUrl = ( + mvId: string, + rmId?: string, + preferredModelRegistry?: string, +): string => `${modelVersionUrl(mvId, rmId, preferredModelRegistry)}/deployments`; diff --git a/clients/ui/frontend/src/app/types.ts b/clients/ui/frontend/src/app/types.ts index 17fcc589..da39d133 100644 --- a/clients/ui/frontend/src/app/types.ts +++ b/clients/ui/frontend/src/app/types.ts @@ -1,4 +1,4 @@ -import { APIOptions } from '~/types'; +import { APIOptions } from '~/app/api/types'; export enum ModelState { LIVE = 'LIVE', @@ -21,6 +21,11 @@ export type ModelRegistry = { description: string; }; +// TODO: Change in the backend AND frontend to "items" instead of "model-registries" +export type ModelRegistryResponse = { + model_registry: ModelRegistry[]; +}; + export enum ModelRegistryMetadataType { INT = 'MetadataIntValue', DOUBLE = 'MetadataDoubleValue', diff --git a/clients/ui/frontend/src/types.ts b/clients/ui/frontend/src/types.ts index 0be5cb1a..34f4c36f 100644 --- a/clients/ui/frontend/src/types.ts +++ b/clients/ui/frontend/src/types.ts @@ -19,14 +19,3 @@ export type CommonConfig = { export type FeatureFlag = { modelRegistry: boolean; }; - -export type APIOptions = { - dryRun?: boolean; - signal?: AbortSignal; - parseJSON?: boolean; -}; - -export type APIError = { - code: string; - message: string; -}; diff --git a/clients/ui/frontend/src/utilities/useFetchState.ts b/clients/ui/frontend/src/utilities/useFetchState.ts index 64b2e3eb..aa688d34 100644 --- a/clients/ui/frontend/src/utilities/useFetchState.ts +++ b/clients/ui/frontend/src/utilities/useFetchState.ts @@ -1,5 +1,5 @@ import * as React from 'react'; -import { APIOptions } from '~/types'; +import { APIOptions } from '~/app/api/types'; /** * Allows "I'm not ready" rejections if you lack a lazy provided prop