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

List connected notebooks for each connection table row #3256

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
1 change: 1 addition & 0 deletions frontend/src/__mocks__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export * from './mockPipelineVersionsProxy';
export * from './mockPipelinesProxy';
export * from './mockDscStatus';
export * from './mockNotebookK8sResource';
export * from './mockNotebookState';
export * from './mockPipelineKF';
export * from './mockSecretK8sResource';
export * from './mockGoogleRpcStatusKF';
Expand Down
30 changes: 30 additions & 0 deletions frontend/src/__mocks__/mockNotebookState.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import _ from 'lodash-es';
import { NotebookKind } from '~/k8sTypes';
import { NotebookState, NotebookRefresh } from '~/pages/projects/notebook/types';

type MockConfigType = {
isStarting?: boolean;
isRunning?: boolean;
isStopping?: boolean;
isStopped?: boolean;
runningPodUid?: string;
refresh?: NotebookRefresh;
};

export const mockNotebookState = (
notebook: NotebookKind,
mockConfig?: MockConfigType,
): NotebookState => ({
notebook,
..._.merge(
{
isStarting: false,
isRunning: false,
isStopping: false,
isStopped: false,
runningPodUid: '',
refresh: () => Promise.resolve(),
},
mockConfig,
),
});
44 changes: 44 additions & 0 deletions frontend/src/concepts/design/TypedObjectIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import * as React from 'react';
import { SVGIconProps } from '@patternfly/react-icons/dist/esm/createIcon';
import { ProjectObjectType, typedColor } from '~/concepts/design/utils';
import NotebookIcon from '~/images/icons/NotebookIcon';
import ModelIcon from '~/images/icons/ModelsIcon';

type TypedObjectIconProps = SVGIconProps & {
resourceType: ProjectObjectType;
useTypedColor?: boolean;
size?: number;
};
const TypedObjectIcon: React.FC<TypedObjectIconProps> = ({
resourceType,
useTypedColor,
style,
...rest

Check warning on line 16 in frontend/src/concepts/design/TypedObjectIcon.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/design/TypedObjectIcon.tsx#L13-L16

Added lines #L13 - L16 were not covered by tests
}) => {
switch (resourceType) {
case ProjectObjectType.notebook:
return (
<NotebookIcon
style={{
color: useTypedColor ? typedColor(resourceType) : undefined,
...(style || {}),
}}
{...rest}
/>
);
case ProjectObjectType.deployedModels:
return (

Check warning on line 30 in frontend/src/concepts/design/TypedObjectIcon.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/design/TypedObjectIcon.tsx#L29-L30

Added lines #L29 - L30 were not covered by tests
<ModelIcon
style={{
color: useTypedColor ? typedColor(resourceType) : undefined,
...(style || {}),

Check warning on line 34 in frontend/src/concepts/design/TypedObjectIcon.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/design/TypedObjectIcon.tsx#L33-L34

Added lines #L33 - L34 were not covered by tests
}}
{...rest}
/>
);
default:
return null;

Check warning on line 40 in frontend/src/concepts/design/TypedObjectIcon.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/design/TypedObjectIcon.tsx#L39-L40

Added lines #L39 - L40 were not covered by tests
}
};

export default TypedObjectIcon;
30 changes: 30 additions & 0 deletions frontend/src/concepts/design/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,36 @@
}
};

export const typedColor = (objectType: ProjectObjectType): string => {
switch (objectType) {
case ProjectObjectType.project:
return 'var(--ai-project--Color)';

Check warning on line 84 in frontend/src/concepts/design/utils.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/design/utils.ts#L83-L84

Added lines #L83 - L84 were not covered by tests
case ProjectObjectType.notebook:
return 'var(--ai-notebook--Color)';
case ProjectObjectType.pipeline:
case ProjectObjectType.pipelineRun:
return 'var(--ai-pipeline--Color)';
case ProjectObjectType.pipelineSetup:
return 'var(--ai-set-up--Color)';
case ProjectObjectType.clusterStorage:
return 'var(--ai-cluster-storage--Color)';
case ProjectObjectType.modelServer:
case ProjectObjectType.registeredModels:
case ProjectObjectType.deployedModels:
case ProjectObjectType.deployingModels:
return 'var(--ai-model-server--Color)';
case ProjectObjectType.dataConnection:
case ProjectObjectType.connections:
return 'var(--ai-data-connection--Color)';
case ProjectObjectType.user:
return 'var(--ai-user--Color)';
case ProjectObjectType.group:
return 'var(--ai-group--Color)';
default:
return '';

Check warning on line 107 in frontend/src/concepts/design/utils.ts

View check run for this annotation

Codecov / codecov/patch

frontend/src/concepts/design/utils.ts#L87-L107

Added lines #L87 - L107 were not covered by tests
}
};

export const typedObjectImage = (objectType: ProjectObjectType): string => {
switch (objectType) {
case ProjectObjectType.project:
Expand Down
17 changes: 17 additions & 0 deletions frontend/src/concepts/design/vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,40 @@
--ai-training--BorderColor: #9ad8d8;
--ai-serving--BorderColor: #92c5f9;


--ai-project--BackgroundColor: var(--ai-organize--BackgroundColor);
--ai-project--BorderColor: var(--ai-organize--BorderColor);
--ai-project--Color: var(--ai-organize--BorderColor);

--ai-data-connection--BackgroundColor: var(--ai-organize--BackgroundColor);
--ai-data-connection--BorderColor: var(--ai-organize--BorderColor);
--ai-data-connection--Color: var(--ai-organize--BorderColor);

--ai-cluster-storage--BackgroundColor: var(--ai-organize--BackgroundColor);
--ai-cluster-storage--BorderColor: var(--ai-organize--BorderColor);
--ai-cluster-storage--Color: var(--ai-organize--BorderColor);

--ai-notebook--BackgroundColor: var(--ai-training--BackgroundColor);
--ai-notebook--BorderColor: var(--ai-training--BorderColor);
--ai-notebook--Color: #37a3a3;

--ai-model--BackgroundColor: var(--ai-serving--BackgroundColor);
--ai-model--BorderColor: var(--ai-serviing--BorderColor);
--ai-model--Color: var(--ai-serviing--BorderColor);

--ai-pipeline--BackgroundColor: var(--ai-training--BackgroundColor);
--ai-pipeline--BorderColor: var(--ai-training--BorderColor);
--ai-pipeline--Color: var(--ai-training--BorderColor);

--ai-model-server--BackgroundColor: var(--ai-serving--BackgroundColor);
--ai-model-server--BorderColor: var(--ai-serving--BorderColor);
--ai-model-server--Color: #5E40BE;

--ai-user--BackgroundColor: var(--ai-set-up--BackgroundColor);
--ai-user--BorderColor: var(--ai-set-up--BorderColor);
--ai-user--Color: var(--ai-set-up--BorderColor);

--ai-group--BackgroundColor: var(--ai-set-up--BackgroundColor);
--ai-group--BorderColor: var(--ai-set-up--BorderColor);
--ai-group--Color: var(--ai-set-up--BorderColor);
}
13 changes: 13 additions & 0 deletions frontend/src/images/icons/ModelsIcon.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { createIcon } from '@patternfly/react-icons/dist/esm/createIcon';

const ModelIcon = createIcon({
name: 'ModelIcon',
width: 32,
height: 32,
svgPath:
'M19.5 11H12.5C11.6729 11 11 10.3271 11 9.5V2.5C11 1.6729 11.6729 1 12.5 1H19.5C20.3271 1 21 1.6729 21 2.5V9.5C21 10.3271 20.3271 11 19.5 11ZM19 3H13V9H19V3ZM12.5 25H5.5C4.6729 25 4 24.3271 4 23.5V16.5C4 15.6729 4.6729 15 5.5 15H12.5C13.3271 15 14 15.6729 14 16.5V23.5C14 24.3271 13.3271 25 12.5 25ZM12 17H6V23H12V17ZM2 29H30C30.5527 29 31 29.4478 31 30C31 30.5522 30.5527 31 30 31H2C1.4473 31 1 30.5522 1 30C1 29.4478 1.4473 29 2 29ZM18 16.5V23.5C18 24.3271 18.6729 25 19.5 25H26.5C27.3271 25 28 24.3271 28 23.5V16.5C28 15.6729 27.3271 15 26.5 15H19.5C18.6729 15 18 15.6729 18 16.5ZM20 17H26V23H20V17Z',
xOffset: 0,
yOffset: 0,
});

export default ModelIcon;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { Label, LabelGroup, Spinner } from '@patternfly/react-core';
import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
import useRelatedNotebooks, { ConnectedNotebookContext } from './useRelatedNotebooks';
import { useRelatedNotebooks, ConnectedNotebookContext } from './useRelatedNotebooks';

type ConnectedNotebookNamesProps = {
context: ConnectedNotebookContext;
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/pages/projects/notebook/useNotebooksStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { POLL_INTERVAL } from '~/utilities/const';
import { getNotebooksStates } from '~/pages/projects/notebook/useProjectNotebookStates';
import { NotebookState } from './types';

const useNotebooksStates = (
export const useNotebooksStates = (
notebooks: NotebookKind[],
namespace: string,
): FetchState<NotebookState[]> => {
Expand All @@ -18,5 +18,3 @@ const useNotebooksStates = (
refreshRate: POLL_INTERVAL,
});
};

export default useNotebooksStates;
4 changes: 1 addition & 3 deletions frontend/src/pages/projects/notebook/useRelatedNotebooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export enum ConnectedNotebookContext {
POSSIBLE_DATA_CONNECTION = 'data-connection-possible',
}

const useRelatedNotebooks = (
export const useRelatedNotebooks = (
context: ConnectedNotebookContext,
resourceName?: string,
): { notebooks: NotebookKind[]; loaded: boolean; error: Error | undefined } => {
Expand Down Expand Up @@ -88,5 +88,3 @@ const useRelatedNotebooks = (
error,
};
};

export default useRelatedNotebooks;
3 changes: 2 additions & 1 deletion frontend/src/pages/projects/pvc/DeletePVCModal.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as React from 'react';
import { PersistentVolumeClaimKind } from '~/k8sTypes';
import { deletePvc, removeNotebookPVC } from '~/api';
import useRelatedNotebooks, {
import {
useRelatedNotebooks,
ConnectedNotebookContext,
} from '~/pages/projects/notebook/useRelatedNotebooks';
import DeleteModal from '~/pages/projects/components/DeleteModal';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import * as React from 'react';
import { LabelGroup, Spinner } from '@patternfly/react-core';
import {
useRelatedNotebooks,
ConnectedNotebookContext,
} from '~/pages/projects/notebook/useRelatedNotebooks';
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';

type Props = {
connection: Connection;
};

const ConnectedResources: React.FC<Props> = ({ connection }) => {
const { notebooks: connectedNotebooks, loaded: notebooksLoaded } = useRelatedNotebooks(
ConnectedNotebookContext.EXISTING_DATA_CONNECTION,
connection.metadata.name,
);

if (!notebooksLoaded) {
return <Spinner size="sm" />;

Check warning on line 23 in frontend/src/pages/projects/screens/detail/connections/ConnectedResources.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/connections/ConnectedResources.tsx#L23

Added line #L23 was not covered by tests
}

if (!connectedNotebooks.length) {
return '-';
}

return (
<LabelGroup>
{connectedNotebooks.map((notebook) => (
<ResourceLabel
key={notebook.metadata.name}
resourceType={ProjectObjectType.notebook}
title={getDisplayNameFromK8sResource(notebook)}
/>
))}
</LabelGroup>
);
};

export default ConnectedResources;
Original file line number Diff line number Diff line change
@@ -1,22 +1,54 @@
import React from 'react';
import { K8sStatus } from '@openshift/dynamic-plugin-sdk-utils';
import {
Badge,
Bullseye,
ExpandableSection,
ExpandableSectionToggle,
Spinner,
TextContent,
TextList,
TextListItem,
} from '@patternfly/react-core';
import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
import { Connection } from '~/concepts/connectionTypes/types';
import DeleteModal from '~/pages/projects/components/DeleteModal';
import {
useRelatedNotebooks,
ConnectedNotebookContext,
} from '~/pages/projects/notebook/useRelatedNotebooks';
import { useNotebooksStates } from '~/pages/projects/notebook/useNotebooksStates';
import { NotebookKind } from '~/k8sTypes';

type Props = {
namespace: string;
deleteConnection: Connection;
onClose: (deleted?: boolean) => void;
onDelete: () => Promise<K8sStatus>;
};

export const ConnectionsDeleteModal: React.FC<Props> = ({
namespace,
deleteConnection,
onClose,
onDelete,
}) => {
const [isDeleting, setIsDeleting] = React.useState(false);
const [error, setError] = React.useState<Error>();
const { notebooks: connectedNotebooks, loaded: notebooksLoaded } = useRelatedNotebooks(
ConnectedNotebookContext.EXISTING_DATA_CONNECTION,
deleteConnection.metadata.name,
);
const [notebookStates] = useNotebooksStates(connectedNotebooks, namespace);
const [notebooksExpanded, setNotebooksExpanded] = React.useState<boolean>(false);

const getNotebookStatusText = React.useCallback(
(notebook: NotebookKind) =>

Check warning on line 46 in frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx#L46

Added line #L46 was not covered by tests
notebookStates.find((n) => n.notebook.metadata.name === notebook.metadata.name)?.isRunning
? ' (Running)'
: '',
[notebookStates],
);

return (
<DeleteModal
Expand All @@ -43,6 +75,48 @@
>
The <b>{getDisplayNameFromK8sResource(deleteConnection)}</b> connection will be deleted, and
its dependent resources will stop working.
{notebooksLoaded && !connectedNotebooks.length ? null : (
<div className="pf-v5-u-mt-md">
{!notebooksLoaded ? (
<Bullseye>

Check warning on line 81 in frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx

View check run for this annotation

Codecov / codecov/patch

frontend/src/pages/projects/screens/detail/connections/ConnectionsDeleteModal.tsx#L81

Added line #L81 was not covered by tests
<Spinner size="md" />
</Bullseye>
) : (
<>
<ExpandableSectionToggle
isExpanded={notebooksExpanded}
onToggle={setNotebooksExpanded}
contentId="expanded-connected-notebooks"
data-testid="connections-delete-notebooks-toggle"
>
<span>Workbenches </span>
<Badge isRead data-testid="connections-delete-notebooks-count">
{connectedNotebooks.length}
</Badge>
</ExpandableSectionToggle>
<ExpandableSection
isExpanded={notebooksExpanded}
isDetached
contentId="expanded-connected-notebooks"
>
<TextContent>
<TextList>
{connectedNotebooks.map((notebook) => (
<TextListItem
key={notebook.metadata.name}
data-testid="connections-delete-notebooks-item"
>
{getDisplayNameFromK8sResource(notebook)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the mocks, I believe we need to append " (Running)" to the display name, if notebook.running === true

{getNotebookStatusText(notebook)}
</TextListItem>
))}
</TextList>
</TextContent>
</ExpandableSection>
</>
)}
</div>
)}
</DeleteModal>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const ConnectionsList: React.FC = () => {
}
>
<ConnectionsTable
namespace={currentProject.metadata.name}
connections={connections}
connectionTypes={connectionTypes}
refreshConnections={refreshConnections}
Expand Down
Loading
Loading