From 9ab10769b8d6741ac44c9a639b6d38a4b8a22031 Mon Sep 17 00:00:00 2001 From: Jeff Phillips Date: Wed, 2 Oct 2024 11:59:16 -0400 Subject: [PATCH] List connected deployed models for each connection table row (#3275) --- .../useInferenceServicesForConnection.spec.ts | 42 ++++++++ .../detail/connections/ConnectedResources.tsx | 9 ++ .../connections/ConnectionsDeleteModal.tsx | 102 +++++++++++++----- .../__tests__/ConnectionsDeleteModal.spec.tsx | 31 +++++- .../__tests__/ConnectionsTable.spec.tsx | 16 +++ .../useInferenceServicesForConnection.ts | 17 +++ 6 files changed, 187 insertions(+), 30 deletions(-) create mode 100644 frontend/src/pages/projects/__tests__/useInferenceServicesForConnection.spec.ts create mode 100644 frontend/src/pages/projects/useInferenceServicesForConnection.ts diff --git a/frontend/src/pages/projects/__tests__/useInferenceServicesForConnection.spec.ts b/frontend/src/pages/projects/__tests__/useInferenceServicesForConnection.spec.ts new file mode 100644 index 0000000000..e29de24954 --- /dev/null +++ b/frontend/src/pages/projects/__tests__/useInferenceServicesForConnection.spec.ts @@ -0,0 +1,42 @@ +import React from 'react'; +import { mockInferenceServiceK8sResource } from '~/__mocks__'; +import { testHook } from '~/__tests__/unit/testUtils/hooks'; +import { useInferenceServicesForConnection } from '~/pages/projects/useInferenceServicesForConnection'; +import { mockConnection } from '~/__mocks__/mockConnection'; + +jest.mock('react', () => ({ + ...jest.requireActual('react'), + useContext: jest.fn(), +})); + +const useContextMock = React.useContext as jest.Mock; + +const connection = mockConnection({ + name: 'connection1', + displayName: 'Connection 1', + description: 'desc1', +}); + +const mockInferenceServices = [ + mockInferenceServiceK8sResource({ + name: 'item-1', + displayName: 'Item 1', + secretName: 'connection1', + }), + mockInferenceServiceK8sResource({ + name: 'item-2', + displayName: 'Item 2', + secretName: 'connection2', + }), +]; + +describe('useInferenceServicesForConnection', () => { + beforeEach(() => { + useContextMock.mockReturnValue({ inferenceServices: { data: mockInferenceServices } }); + }); + + it('should return matching inference services', () => { + const renderResult = testHook(useInferenceServicesForConnection)(connection); + expect(renderResult.result.current).toHaveLength(1); + }); +}); diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectedResources.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectedResources.tsx index b0a44e60e4..433e33c41c 100644 --- a/frontend/src/pages/projects/screens/detail/connections/ConnectedResources.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/ConnectedResources.tsx @@ -8,6 +8,7 @@ import { Connection } from '~/concepts/connectionTypes/types'; import { ProjectObjectType } from '~/concepts/design/utils'; import ResourceLabel from '~/pages/projects/screens/detail/connections/ResourceLabel'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import { useInferenceServicesForConnection } from '~/pages/projects/useInferenceServicesForConnection'; type Props = { connection: Connection; @@ -18,6 +19,7 @@ const ConnectedResources: React.FC = ({ connection }) => { ConnectedNotebookContext.EXISTING_DATA_CONNECTION, connection.metadata.name, ); + const connectedModels = useInferenceServicesForConnection(connection); if (!notebooksLoaded) { return ; @@ -36,6 +38,13 @@ const ConnectedResources: React.FC = ({ connection }) => { title={getDisplayNameFromK8sResource(notebook)} /> ))} + {connectedModels.map((model) => ( + + ))} ); }; diff --git a/frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx b/frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx index 6697614bbe..45b2b208ad 100644 --- a/frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx @@ -19,6 +19,7 @@ import { } from '~/pages/projects/notebook/useRelatedNotebooks'; import { useNotebooksStates } from '~/pages/projects/notebook/useNotebooksStates'; import { NotebookKind } from '~/k8sTypes'; +import { useInferenceServicesForConnection } from '~/pages/projects/useInferenceServicesForConnection'; type Props = { namespace: string; @@ -35,12 +36,14 @@ export const ConnectionsDeleteModal: React.FC = ({ }) => { const [isDeleting, setIsDeleting] = React.useState(false); const [error, setError] = React.useState(); - const { notebooks: connectedNotebooks, loaded: notebooksLoaded } = useRelatedNotebooks( + const { notebooks: connectedNotebooks, loaded } = useRelatedNotebooks( ConnectedNotebookContext.EXISTING_DATA_CONNECTION, deleteConnection.metadata.name, ); const [notebookStates] = useNotebooksStates(connectedNotebooks, namespace); const [notebooksExpanded, setNotebooksExpanded] = React.useState(false); + const connectedModels = useInferenceServicesForConnection(deleteConnection); + const [modelsExpanded, setModelsExpanded] = React.useState(false); const getNotebookStatusText = React.useCallback( (notebook: NotebookKind) => @@ -75,44 +78,85 @@ export const ConnectionsDeleteModal: React.FC = ({ > The {getDisplayNameFromK8sResource(deleteConnection)} connection will be deleted, and its dependent resources will stop working. - {notebooksLoaded && !connectedNotebooks.length ? null : ( + {loaded && !connectedNotebooks.length && !connectedModels.length ? null : (
- {!notebooksLoaded ? ( + {!loaded ? ( ) : ( <> + {connectedNotebooks.length ? ( + <> + + Workbenches + + {connectedNotebooks.length} + + + {notebooksExpanded ? ( + + + + {connectedNotebooks.map((notebook) => ( + + {getDisplayNameFromK8sResource(notebook)} + {getNotebookStatusText(notebook)} + + ))} + + + + ) : null} + + ) : null} - Workbenches - - {connectedNotebooks.length} + Model deployments + + {connectedModels.length} - - - - {connectedNotebooks.map((notebook) => ( - - {getDisplayNameFromK8sResource(notebook)} - {getNotebookStatusText(notebook)} - - ))} - - - + {modelsExpanded ? ( + + + + {connectedModels.map((model) => ( + + {getDisplayNameFromK8sResource(model)} + + ))} + + + + ) : null} )}
diff --git a/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsDeleteModal.spec.tsx b/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsDeleteModal.spec.tsx index f1d7fc5975..874dae3d05 100644 --- a/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsDeleteModal.spec.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsDeleteModal.spec.tsx @@ -4,8 +4,13 @@ import { fireEvent, render, screen } from '@testing-library/react'; import { ConnectionsDeleteModal } from '~/pages/projects/screens/detail/connections/ConnectionsDeleteModal'; import { mockConnection } from '~/__mocks__/mockConnection'; import { useRelatedNotebooks } from '~/pages/projects/notebook/useRelatedNotebooks'; -import { mockNotebookK8sResource, mockNotebookState } from '~/__mocks__'; +import { + mockInferenceServiceK8sResource, + mockNotebookK8sResource, + mockNotebookState, +} from '~/__mocks__'; import { useNotebooksStates } from '~/pages/projects/notebook/useNotebooksStates'; +import { useInferenceServicesForConnection } from '~/pages/projects/useInferenceServicesForConnection'; jest.mock('~/pages/projects/notebook/useRelatedNotebooks', () => ({ ...jest.requireActual('~/pages/projects/notebook/useRelatedNotebooks'), @@ -16,8 +21,13 @@ jest.mock('~/pages/projects/notebook/useNotebooksStates', () => ({ useNotebooksStates: jest.fn(), })); +jest.mock('~/pages/projects/useInferenceServicesForConnection', () => ({ + useInferenceServicesForConnection: jest.fn(), +})); + const useRelatedNotebooksMock = useRelatedNotebooks as jest.Mock; const useNotebooksStatesMock = useNotebooksStates as jest.Mock; +const useInferenceServicesForConnectionMock = useInferenceServicesForConnection as jest.Mock; const mockNotebooks = [ mockNotebookK8sResource({ name: 'connected-notebook', displayName: 'Connected notebook' }), @@ -38,6 +48,16 @@ describe('Delete connection modal', () => { mockNotebookState(mockNotebooks[1], { isStopped: true }), ], ]); + useInferenceServicesForConnectionMock.mockReturnValue([ + mockInferenceServiceK8sResource({ + name: 'deployed-model-1', + displayName: 'Deployed model 1', + }), + mockInferenceServiceK8sResource({ + name: 'deployed-model-2', + displayName: 'Deployed model 2', + }), + ]); }); it('should show related resources', async () => { @@ -60,5 +80,14 @@ describe('Delete connection modal', () => { expect(notebookItems).toHaveLength(2); expect(notebookItems[0]).toHaveTextContent('Connected notebook (Running)'); expect(notebookItems[1]).toHaveTextContent('Another notebook'); + + const modelsCountBadge = screen.getByTestId('connections-delete-models-count'); + expect(modelsCountBadge).toHaveTextContent('2'); + await act(() => fireEvent.click(modelsCountBadge)); + + const modelsItems = screen.getAllByTestId('connections-delete-models-item'); + expect(modelsItems).toHaveLength(2); + expect(modelsItems[0]).toHaveTextContent('Deployed model 1'); + expect(modelsItems[1]).toHaveTextContent('Deployed model 2'); }); }); diff --git a/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx b/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx index 89b52b7515..2877da8c61 100644 --- a/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx +++ b/frontend/src/pages/projects/screens/detail/connections/__tests__/ConnectionsTable.spec.tsx @@ -5,17 +5,30 @@ import { mockConnectionTypeConfigMapObj } from '~/__mocks__/mockConnectionType'; import { mockConnection } from '~/__mocks__/mockConnection'; import { mockNotebookK8sResource } from '~/__mocks__/mockNotebookK8sResource'; import { useRelatedNotebooks } from '~/pages/projects/notebook/useRelatedNotebooks'; +import { useInferenceServicesForConnection } from '~/pages/projects/useInferenceServicesForConnection'; +import { mockInferenceServiceK8sResource } from '~/__mocks__'; jest.mock('~/pages/projects/notebook/useRelatedNotebooks', () => ({ ...jest.requireActual('~/pages/projects/notebook/useRelatedNotebooks'), useRelatedNotebooks: jest.fn(), })); +jest.mock('~/pages/projects/useInferenceServicesForConnection', () => ({ + useInferenceServicesForConnection: jest.fn(), +})); + const useRelatedNotebooksMock = useRelatedNotebooks as jest.Mock; +const useInferenceServicesForConnectionMock = useInferenceServicesForConnection as jest.Mock; + +const mockInferenceServices = [ + mockInferenceServiceK8sResource({ name: 'deployed-model-1', displayName: 'Deployed model 1' }), + mockInferenceServiceK8sResource({ name: 'deployed-model-2', displayName: 'Deployed model 2' }), +]; describe('ConnectionsTable', () => { beforeEach(() => { useRelatedNotebooksMock.mockReturnValue({ notebooks: [], loaded: true }); + useInferenceServicesForConnectionMock.mockReturnValue([]); }); it('should render table', () => { @@ -61,6 +74,7 @@ describe('ConnectionsTable', () => { notebooks: [mockNotebookK8sResource({ displayName: 'Connected notebook' })], loaded: true, }); + useInferenceServicesForConnectionMock.mockReturnValue(mockInferenceServices); const connection = mockConnection({ displayName: 'connection1', description: 'desc1' }); render( @@ -81,5 +95,7 @@ describe('ConnectionsTable', () => { expect(screen.queryByText('s3')).toBeFalsy(); expect(screen.getByText('S3 Buckets')).toBeTruthy(); expect(screen.getByText('Connected notebook')).toBeTruthy(); + expect(screen.getByText('Deployed model 1')).toBeTruthy(); + expect(screen.getByText('Deployed model 2')).toBeTruthy(); }); }); diff --git a/frontend/src/pages/projects/useInferenceServicesForConnection.ts b/frontend/src/pages/projects/useInferenceServicesForConnection.ts new file mode 100644 index 0000000000..907a14db80 --- /dev/null +++ b/frontend/src/pages/projects/useInferenceServicesForConnection.ts @@ -0,0 +1,17 @@ +import * as React from 'react'; +import { InferenceServiceKind } from '~/k8sTypes'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext'; + +export const useInferenceServicesForConnection = ( + connection: Connection, +): InferenceServiceKind[] => { + const { + inferenceServices: { data: inferenceServices }, + } = React.useContext(ProjectDetailsContext); + const connectionName = connection.metadata.name; + + return inferenceServices.filter( + (inferenceService) => inferenceService.spec.predictor.model?.storage?.key === connectionName, + ); +};