Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Backport 2.x] update preview panel for external models #253

Merged
merged 1 commit into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import React from 'react';
import userEvent from '@testing-library/user-event';
import { render, screen } from '../../../../test/test_utils';
import { ConnectorDetails } from '../connector_details';

function setup({ name = 'name', id = 'id', description = 'description' }) {
const user = userEvent.setup({});
render(<ConnectorDetails name={name} id={id} description={description} />);
return { user };
}

describe('<ConnectorDetails />', () => {
it('should render connector details', () => {
setup({});
expect(screen.getByText('Connector name')).toBeInTheDocument();
expect(screen.getByText('Connector ID')).toBeInTheDocument();
expect(screen.getByText('Connector description')).toBeInTheDocument();
});

it('should render - when id is empty', () => {
setup({ id: '' });
expect(screen.getByText('-')).toBeInTheDocument();
expect(screen.queryByTestId('copyable-text-div')).not.toBeInTheDocument();
});

it('should render id and copy id button when id is not empty', () => {
setup({ id: 'connector-id' });
expect(screen.getByText('connector-id')).toBeInTheDocument();
expect(screen.queryByTestId('copyable-text-div')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ const NODES = [
},
];

function setup({ nodes = NODES, loading = false }) {
function setup({ nodes = NODES, loading = false, nodesStatus = 'Responding on 1 of 2 nodes' }) {
const user = userEvent.setup({});
render(<NodesTable nodes={nodes} loading={loading} />);
render(<NodesTable nodes={nodes} loading={loading} nodesStatus={nodesStatus} />);
return { user };
}

Expand All @@ -31,6 +31,7 @@ describe('<NodesTable />', () => {
expect(screen.getAllByRole('columnheader').length).toBe(2);
expect(screen.getByText('id1')).toBeInTheDocument();
expect(screen.getByText('id2')).toBeInTheDocument();
expect(screen.getByText('Responding on 1 of 2 nodes')).toBeInTheDocument();
});

it('should render status at first column with asc by default', () => {
Expand Down
33 changes: 25 additions & 8 deletions public/components/preview_panel/__tests__/preview_panel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ const MODEL = {
id: 'id1',
name: 'test',
planningWorkerNodes: ['node-1', 'node-2', 'node-3'],
connector: {
name: 'Connector',
},
};

function setup({ model = MODEL, onClose = jest.fn() }) {
Expand All @@ -29,11 +26,30 @@ describe('<PreviewPanel />', () => {
jest.clearAllMocks();
});

it('should render id, name and source in panel', () => {
it('should render id, name in panel', () => {
setup({});
expect(screen.getByText('test')).toBeInTheDocument();
expect(screen.getByText('id1')).toBeInTheDocument();
});

it('source should be local and should not render connector details when no connector params passed', async () => {
setup({});
expect(screen.getByText('Local')).toBeInTheDocument();
expect(screen.queryByText('Connector details')).not.toBeInTheDocument();
});

it('source should be external and should not render nodes details when connector params passed', async () => {
const modelWithConntector = {
...MODEL,
connector: {
name: 'connector',
},
};
setup({
model: modelWithConntector,
});
expect(screen.getByText('External')).toBeInTheDocument();
expect(screen.queryByText('Status by node')).not.toBeInTheDocument();
});

it('should call onClose when close panel', async () => {
Expand All @@ -44,7 +60,7 @@ describe('<PreviewPanel />', () => {
expect(onClose).toHaveBeenCalled();
});

it('should render loading when not responding and render partially state when responding', async () => {
it('should render loading when local model not responding and render partially state when responding', async () => {
const request = jest.spyOn(APIProvider.getAPI('profile'), 'getModel');
const mockResult = {
id: 'model-1-id',
Expand All @@ -55,9 +71,10 @@ describe('<PreviewPanel />', () => {
request.mockResolvedValue(mockResult);
setup({});
expect(screen.getByTestId('preview-panel-color-loading-text')).toBeInTheDocument();
await waitFor(() =>
expect(screen.getByText('Partially responding on 2 of 3 nodes')).toBeInTheDocument()
);
await waitFor(() => {
expect(screen.getByText('Partially responding')).toBeInTheDocument();
expect(screen.getByText('Responding on 2 of 3 nodes')).toBeInTheDocument();
});
});

it('should render not responding when no model profile API response', async () => {
Expand Down
62 changes: 62 additions & 0 deletions public/components/preview_panel/connector_details.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import React from 'react';
import {
EuiDescriptionList,
EuiDescriptionListTitle,
EuiTitle,
EuiSpacer,
EuiDescriptionListDescription,
EuiFlexGroup,
EuiFlexItem,
} from '@elastic/eui';
import { CopyableText } from '../common';

export const ConnectorDetails = (props: { name?: string; id?: string; description?: string }) => {
const { name, id, description } = props;
return (
<>
<EuiSpacer size="m" />
<EuiTitle size="s">
<h3>Connector details</h3>
</EuiTitle>
<EuiSpacer size="m" />
<EuiDescriptionList>
<EuiFlexGroup>
<EuiFlexItem>
<EuiDescriptionListTitle>
<EuiTitle size="xxs">
<h5>Connector name</h5>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>{name}</EuiDescriptionListDescription>
</EuiFlexItem>
<EuiFlexItem>
<EuiDescriptionListTitle>
<EuiTitle size="xxs">
<h5>Connector ID</h5>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>
{id ? (
<CopyableText text={id} iconLeft={false} tooltipText="Copy connector ID" />
) : (
'-'
)}
</EuiDescriptionListDescription>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="m" />
<EuiDescriptionListTitle>
<EuiTitle size="xxs">
<h5>Connector description</h5>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>{description}</EuiDescriptionListDescription>
</EuiDescriptionList>
</>
);
};
107 changes: 75 additions & 32 deletions public/components/preview_panel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@ import {
EuiDescriptionListDescription,
EuiSpacer,
EuiTextColor,
EuiFlexGroup,
EuiFlexItem,
} from '@elastic/eui';
import { APIProvider } from '../../apis/api_provider';
import { useFetcher } from '../../hooks/use_fetcher';
import { NodesTable } from './nodes_table';
import { CopyableText } from '../common';
import { ModelDeploymentProfile } from '../../apis/profile';
import { ConnectorDetails } from './connector_details';

export interface INode {
id: string;
Expand Down Expand Up @@ -60,33 +63,45 @@ export const PreviewPanel = ({ onClose, model }: Props) => {

const respondingStatus = useMemo(() => {
if (loading) {
return (
<EuiTextColor color="subdued" data-test-subj="preview-panel-color-loading-text">
Loading...
</EuiTextColor>
);
return {
overall: (
<EuiTextColor color="subdued" data-test-subj="preview-panel-color-loading-text">
Loading...
</EuiTextColor>
),
nodes: 'Loading...',
};
}
const deployedNodesNum = nodes.filter(({ deployed }) => deployed).length;
const targetNodesNum = nodes.length;
if (deployedNodesNum === 0) {
return (
<EuiHealth className="ml-modelStatusCell" color="danger">
Not responding on {targetNodesNum} of {targetNodesNum} nodes
</EuiHealth>
);
return {
overall: (
<EuiHealth className="ml-modelStatusCell" color="danger">
Not responding
</EuiHealth>
),
nodes: `Not responding on ${targetNodesNum} of ${targetNodesNum} nodes`,
};
}
if (deployedNodesNum < targetNodesNum) {
return (
<EuiHealth className="ml-modelStatusCell" color="warning">
Partially responding on {deployedNodesNum} of {targetNodesNum} nodes
</EuiHealth>
);
return {
overall: (
<EuiHealth className="ml-modelStatusCell" color="warning">
Partially responding
</EuiHealth>
),
nodes: `Responding on ${deployedNodesNum} of ${targetNodesNum} nodes`,
};
}
return (
<EuiHealth className="ml-modelStatusCell" color="success">
Responding on {deployedNodesNum} of {targetNodesNum} nodes
</EuiHealth>
);
return {
overall: (
<EuiHealth className="ml-modelStatusCell" color="success">
Responding
</EuiHealth>
),
nodes: `Responding on ${deployedNodesNum} of ${targetNodesNum} nodes`,
};
}, [nodes, loading]);

const onCloseFlyout = useCallback(() => {
Expand All @@ -95,26 +110,54 @@ export const PreviewPanel = ({ onClose, model }: Props) => {

return (
<EuiFlyout onClose={onCloseFlyout}>
<EuiFlyoutHeader>
<EuiTitle size="s">
<h3>{name}</h3>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2>{name}</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiDescriptionList>
<EuiDescriptionListTitle>Model ID</EuiDescriptionListTitle>
<EuiFlexGroup>
<EuiFlexItem>
<EuiDescriptionListTitle>
<EuiTitle size="xxs">
<h5>Status</h5>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>
{respondingStatus.overall}
</EuiDescriptionListDescription>
</EuiFlexItem>
<EuiFlexItem>
<EuiDescriptionListTitle>
<EuiTitle size="xxs">
<h5>Source</h5>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>
{connector ? 'External' : 'Local'}
</EuiDescriptionListDescription>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="m" />
<EuiDescriptionListTitle>
<EuiTitle size="xxs">
<h5>Model ID</h5>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>
<CopyableText text={id} iconLeft={false} tooltipText="Copy model ID" />
</EuiDescriptionListDescription>
<EuiDescriptionListTitle>Source</EuiDescriptionListTitle>
<EuiDescriptionListDescription>
{connector ? 'External' : 'Local'}
</EuiDescriptionListDescription>
<EuiDescriptionListTitle>Model status by node</EuiDescriptionListTitle>
<EuiDescriptionListDescription>{respondingStatus}</EuiDescriptionListDescription>
</EuiDescriptionList>
<EuiSpacer />
<NodesTable loading={loading} nodes={nodes} />
{connector ? (
<ConnectorDetails
id={connector.id}
name={connector.name}
description={connector.description}
/>
) : (
<NodesTable loading={loading} nodes={nodes} nodesStatus={respondingStatus.nodes} />
)}
</EuiFlyoutBody>
</EuiFlyout>
);
Expand Down
43 changes: 30 additions & 13 deletions public/components/preview_panel/nodes_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@ import {
EuiEmptyPrompt,
EuiCopy,
EuiText,
EuiDescriptionList,
EuiDescriptionListTitle,
EuiTitle,
EuiSpacer,
EuiDescriptionListDescription,
} from '@elastic/eui';
import { INode } from './';

export function NodesTable(props: { nodes: INode[]; loading: boolean }) {
const { nodes, loading } = props;
export function NodesTable(props: { nodes: INode[]; loading: boolean; nodesStatus: string }) {
const { nodes, loading, nodesStatus } = props;
const [sort, setSort] = useState<{ field: keyof INode; direction: Direction }>({
field: 'deployed',
direction: 'asc',
Expand Down Expand Up @@ -108,16 +113,28 @@ export function NodesTable(props: { nodes: INode[]; loading: boolean }) {
);

return (
<EuiBasicTable<INode>
columns={columns}
items={items}
sorting={{ sort }}
pagination={pagination}
onChange={handleTableChange}
loading={loading}
noItemsMessage={
loading ? <EuiEmptyPrompt body={<>Loading...</>} aria-label="loading nodes" /> : undefined
}
/>
<>
<EuiSpacer size="l" />
<EuiDescriptionList>
<EuiDescriptionListTitle>
<EuiTitle size="s">
<h3>Status by node</h3>
</EuiTitle>
</EuiDescriptionListTitle>
<EuiDescriptionListDescription>{nodesStatus}</EuiDescriptionListDescription>
</EuiDescriptionList>
<EuiSpacer size="m" />
<EuiBasicTable<INode>
columns={columns}
items={items}
sorting={{ sort }}
pagination={pagination}
onChange={handleTableChange}
loading={loading}
noItemsMessage={
loading ? <EuiEmptyPrompt body={<>Loading...</>} aria-label="loading nodes" /> : undefined
}
/>
</>
);
}