Add source field to distinguish local and external model (#239) (#241)

* Revert "feat: exclude remote model for admin UI (#225)" This reverts commit ba01d34. Signed-off-by: Lin Wang <wonglam@amazon.com> * feat: add source field to distinguish local and external model Signed-off-by: Lin Wang <wonglam@amazon.com> * feat: add miss display words Signed-off-by: Lin Wang <wonglam@amazon.com> --------- Signed-off-by: Lin Wang <wonglam@amazon.com> (cherry picked from commit 63c7a5a) Co-authored-by: Lin Wang <wonglam@amazon.com>
opensearch-project · Aug 2, 2023 · ec30768 · ec30768
1 parent f542649
commit ec30768
Show file tree

Hide file tree

Showing 10 changed files with 94 additions and 49 deletions.
diff --git a/public/apis/model.ts b/public/apis/model.ts
@@ -30,7 +30,6 @@ export class Model {
     size: number;
     states?: MODEL_STATE[];
     nameOrId?: string;
-    exclude?: 'REMOTE_MODEL';
   }) {
     return InnerHttpProvider.getHttp().get<ModelSearchResponse>(MODEL_API_ENDPOINT, {
       query,

diff --git a/public/components/monitoring/model_deployment_table.tsx b/public/components/monitoring/model_deployment_table.tsx
@@ -80,7 +80,7 @@ export const ModelDeploymentTable = ({
       {
         field: 'model_state',
         name: 'Status',
-        width: '37.5%',
+        width: '34.5%',
         sortable: true,
         truncateText: true,
         render: (
@@ -124,10 +124,17 @@ export const ModelDeploymentTable = ({
           );
         },
       },
+      {
+        field: 'source',
+        name: 'Source',
+        width: '7%',
+        sortable: false,
+        truncateText: true,
+      },
       {
         field: 'id',
         name: 'Model ID',
-        width: '25%',
+        width: '21%',
         sortable: true,
         render: (id: string) => (
           <EuiCopy

diff --git a/public/components/monitoring/tests/model_deployment_table.test.tsx b/public/components/monitoring/tests/model_deployment_table.test.tsx
@@ -19,6 +19,7 @@ const setup = (props?: Partial<ModelDeploymentTableProps>) => {
         notRespondingNodesCount: 2,
         planningNodesCount: 3,
         planningWorkerNodes: [],
+        source: 'Local',
       },
       {
         id: 'model-2-id',
@@ -27,6 +28,7 @@ const setup = (props?: Partial<ModelDeploymentTableProps>) => {
         notRespondingNodesCount: 0,
         planningNodesCount: 3,
         planningWorkerNodes: [],
+        source: 'Local',
       },
       {
         id: 'model-3-id',
@@ -35,6 +37,7 @@ const setup = (props?: Partial<ModelDeploymentTableProps>) => {
         notRespondingNodesCount: 3,
         planningNodesCount: 3,
         planningWorkerNodes: [],
+        source: 'External',
       },
     ],
     pagination: { currentPage: 1, pageSize: 10, totalRecords: 100 },
@@ -122,11 +125,26 @@ describe('<DeployedModelTable />', () => {
       expect(within(cells[2] as HTMLElement).getByText('on 3 of 3 nodes')).toBeInTheDocument();
     });
 
-    it('should render Model ID at third column and copy to clipboard after text clicked', async () => {
+    it('should display source name at third column', () => {
+      const columnIndex = 2;
+      setup();
+      const header = screen.getAllByRole('columnheader')[columnIndex];
+      const columnContent = header
+        .closest('table')
+        ?.querySelectorAll(`tbody tr td:nth-child(${columnIndex + 1})`);
+      expect(within(header).getByText('Source')).toBeInTheDocument();
+      expect(columnContent?.length).toBe(3);
+      const cells = columnContent!;
+      expect(within(cells[0] as HTMLElement).getByText('Local')).toBeInTheDocument();
+      expect(within(cells[1] as HTMLElement).getByText('Local')).toBeInTheDocument();
+      expect(within(cells[2] as HTMLElement).getByText('External')).toBeInTheDocument();
+    });
+
+    it('should render Model ID at forth column and copy to clipboard after text clicked', async () => {
       const execCommandOrigin = document.execCommand;
       document.execCommand = jest.fn(() => true);
 
-      const columnIndex = 2;
+      const columnIndex = 3;
       setup();
       const header = screen.getAllByRole('columnheader')[columnIndex];
       const columnContent = header
@@ -146,7 +164,7 @@ describe('<DeployedModelTable />', () => {
     });
 
     it('should render Action column and call onViewDetail with the model item of the current table row', async () => {
-      const columnIndex = 3;
+      const columnIndex = 4;
       const onViewDetailMock = jest.fn();
       const { finalProps } = setup({
         onViewDetail: onViewDetailMock,
@@ -258,7 +276,7 @@ describe('<DeployedModelTable />', () => {
       },
     });
 
-    await userEvent.click(within(screen.getAllByRole('columnheader')[2]).getByText('Model ID'));
+    await userEvent.click(within(screen.getAllByRole('columnheader')[3]).getByText('Model ID'));
     expect(finalProps.onChange).toHaveBeenCalledWith(
       expect.objectContaining({
         sort: {
@@ -277,7 +295,7 @@ describe('<DeployedModelTable />', () => {
         }}
       />
     );
-    await userEvent.click(within(screen.getAllByRole('columnheader')[2]).getByText('Model ID'));
+    await userEvent.click(within(screen.getAllByRole('columnheader')[3]).getByText('Model ID'));
     expect(finalProps.onChange).toHaveBeenCalledWith(
       expect.objectContaining({
         sort: {

diff --git a/public/components/monitoring/tests/use_monitoring.test.ts b/public/components/monitoring/tests/use_monitoring.test.ts
@@ -124,6 +124,54 @@ describe('useMonitoring', () => {
     });
     await waitFor(() => expect(Model.prototype.search).toHaveBeenCalledTimes(2));
   });
+
+  it('should return consistent deployedModels', async () => {
+    jest.spyOn(Model.prototype, 'search').mockRestore();
+    const searchMock = jest.spyOn(Model.prototype, 'search').mockResolvedValue({
+      data: [
+        {
+          id: 'model-1-id',
+          name: 'model-1-name',
+          current_worker_node_count: 1,
+          planning_worker_node_count: 3,
+          algorithm: 'TEXT_EMBEDDING',
+          model_state: '',
+          model_version: '',
+          planning_worker_nodes: ['node1', 'node2', 'node3'],
+        },
+        {
+          id: 'model-2-id',
+          name: 'model-2-name',
+          current_worker_node_count: 1,
+          planning_worker_node_count: 3,
+          algorithm: 'REMOTE',
+          model_state: '',
+          model_version: '',
+          planning_worker_nodes: ['node1', 'node2', 'node3'],
+        },
+      ],
+      total_models: 2,
+    });
+    const { result, waitFor } = renderHook(() => useMonitoring());
+
+    await waitFor(() => {
+      expect(result.current.deployedModels).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            id: 'model-1-id',
+            name: 'model-1-name',
+            respondingNodesCount: 1,
+            notRespondingNodesCount: 2,
+            planningNodesCount: 3,
+            planningWorkerNodes: ['node1', 'node2', 'node3'],
+          }),
+          expect.objectContaining({ source: 'External' }),
+        ])
+      );
+    });
+
+    searchMock.mockRestore();
+  });
 });
 
 describe('useMonitoring.pageStatus', () => {
@@ -195,21 +243,4 @@ describe('useMonitoring.pageStatus', () => {
 
     await waitFor(() => expect(result.current.pageStatus).toBe('empty'));
   });
-
-  it('should return consistent deployedModels', async () => {
-    const { result, waitFor } = renderHook(() => useMonitoring());
-
-    await waitFor(() =>
-      expect(result.current.deployedModels).toEqual([
-        {
-          id: 'model-1-id',
-          name: 'model-1-name',
-          respondingNodesCount: 1,
-          notRespondingNodesCount: 2,
-          planningNodesCount: 3,
-          planningWorkerNodes: ['node1', 'node2', 'node3'],
-        },
-      ])
-    );
-  });
 });
diff --git a/public/components/monitoring/use_monitoring.ts b/public/components/monitoring/use_monitoring.ts
@@ -47,7 +47,6 @@ const fetchDeployedModels = async (params: Params) => {
         ? [MODEL_STATE.loadFailed, MODEL_STATE.loaded, MODEL_STATE.partiallyLoaded]
         : states,
     sort: [`${params.sort.field}-${params.sort.direction}`],
-    exclude: 'REMOTE_MODEL',
   });
   const totalPages = Math.ceil(result.total_models / params.pageSize);
   return {
@@ -64,6 +63,7 @@ const fetchDeployedModels = async (params: Params) => {
         current_worker_node_count: workerCount,
         planning_worker_node_count: planningCount,
         planning_worker_nodes: planningWorkerNodes,
+        algorithm,
       }) => {
         return {
           id,
@@ -75,6 +75,7 @@ const fetchDeployedModels = async (params: Params) => {
               ? planningCount - workerCount
               : undefined,
           planningWorkerNodes,
+          source: algorithm === 'REMOTE' ? 'External' : 'Local',
         };
       }
     ),

diff --git a/public/components/preview_panel/__tests__/preview_panel.test.tsx b/public/components/preview_panel/__tests__/preview_panel.test.tsx
@@ -13,6 +13,7 @@ const MODEL = {
   id: 'id1',
   name: 'test',
   planningWorkerNodes: ['node-1', 'node-2', 'node-3'],
+  source: 'External',
 };
 
 function setup({ model = MODEL, onClose = jest.fn() }) {
@@ -26,10 +27,11 @@ describe('<PreviewPanel />', () => {
     jest.clearAllMocks();
   });
 
-  it('should render id and name in panel', () => {
+  it('should render id, name and source in panel', () => {
     setup({});
     expect(screen.getByText('test')).toBeInTheDocument();
     expect(screen.getByText('id1')).toBeInTheDocument();
+    expect(screen.getByText('External')).toBeInTheDocument();
   });
 
   it('should call onClose when close panel', async () => {

diff --git a/public/components/preview_panel/index.tsx b/public/components/preview_panel/index.tsx
@@ -31,6 +31,7 @@ export interface PreviewModel {
   name: string;
   id: string;
   planningWorkerNodes: string[];
+  source: string;
 }
 
 interface Props {
@@ -39,7 +40,7 @@ interface Props {
 }
 
 export const PreviewPanel = ({ onClose, model }: Props) => {
-  const { id, name } = model;
+  const { id, name, source } = model;
   const { data, loading } = useFetcher(APIProvider.getAPI('profile').getModel, id);
   const nodes = useMemo(() => {
     if (loading) {
@@ -101,6 +102,8 @@ export const PreviewPanel = ({ onClose, model }: Props) => {
           <EuiDescriptionListDescription>
             <CopyableText text={id} iconLeft={false} tooltipText="Copy model ID" />
           </EuiDescriptionListDescription>
+          <EuiDescriptionListTitle style={{ fontSize: '14px' }}>Source</EuiDescriptionListTitle>
+          <EuiDescriptionListDescription>{source}</EuiDescriptionListDescription>
           <EuiDescriptionListTitle style={{ fontSize: '14px' }}>
             Model status by node
           </EuiDescriptionListTitle>

diff --git a/server/routes/model_router.ts b/server/routes/model_router.ts
@@ -42,12 +42,11 @@ export const modelRouter = (router: IRouter) => {
           ),
           states: schema.maybe(schema.oneOf([schema.arrayOf(modelStateSchema), modelStateSchema])),
           nameOrId: schema.maybe(schema.string()),
-          exclude: schema.maybe(schema.literal('REMOTE_MODEL')),
         }),
       },
     },
     async (context, request) => {
-      const { from, size, sort, states, nameOrId, exclude } = request.query;
+      const { from, size, sort, states, nameOrId } = request.query;
       try {
         const payload = await ModelService.search({
           client: context.core.opensearch.client,
@@ -56,7 +55,6 @@ export const modelRouter = (router: IRouter) => {
           sort: typeof sort === 'string' ? [sort] : sort,
           states: typeof states === 'string' ? [states] : states,
           nameOrId,
-          exclude,
         });
         return opensearchDashboardsResponseFactory.ok({ body: payload });
       } catch (err) {

diff --git a/server/services/model_service.ts b/server/services/model_service.ts
@@ -43,7 +43,6 @@ export class ModelService {
     sort?: ModelSearchSort[];
     states?: MODEL_STATE[];
     nameOrId?: string;
-    exclude?: 'REMOTE_MODEL';
   }) {
     const {
       body: { hits },

diff --git a/server/services/utils/model.ts b/server/services/utils/model.ts
@@ -9,11 +9,9 @@ import { generateTermQuery } from './query';
 export const generateModelSearchQuery = ({
   states,
   nameOrId,
-  exclude,
 }: {
   states?: MODEL_STATE[];
   nameOrId?: string;
-  exclude?: 'REMOTE_MODEL';
 }) => ({
   bool: {
     must: [
@@ -35,21 +33,10 @@ export const generateModelSearchQuery = ({
           ]
         : []),
     ],
-    must_not: [
-      {
-        exists: {
-          field: 'chunk_number',
-        },
+    must_not: {
+      exists: {
+        field: 'chunk_number',
       },
-      ...(exclude === 'REMOTE_MODEL'
-        ? [
-            {
-              term: {
-                algorithm: 'REMOTE',
-              },
-            },
-          ]
-        : []),
-    ],
+    },
   },
 });