From c7dcbd74f1cc805095a1ddf8d42dae30bdecfe3c Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Tue, 26 Nov 2024 22:07:35 +0530 Subject: [PATCH 01/16] Add autofill modal and button for URI connections --- .../screens/RegisterModel/ConnectionModal.tsx | 12 ++- .../RegistrationCommonFormSections.tsx | 82 +++++++++++++------ 2 files changed, 65 insertions(+), 29 deletions(-) diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx index 53febd8466..4fc8281f71 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx @@ -3,14 +3,18 @@ import { Button, FormGroup, HelperText, Form, FormHelperText } from '@patternfly import { Modal } from '@patternfly/react-core/deprecated'; import ProjectSelector from '~/concepts/projects/ProjectSelector'; import { DataConnection } from '~/pages/projects/types'; +import { Connection } from '~/concepts/connectionTypes/types'; import { ConnectionDropdown } from './ConnectionDropdown'; +import { ModelLocationType } from './useRegisterModelData'; export const ConnectionModal: React.FC<{ + type: ModelLocationType; onClose: () => void; - onSubmit: (connection: DataConnection) => void; -}> = ({ onClose, onSubmit }) => { + onSubmit: (connection: DataConnection | Connection) => void; +}> = ({ type, onClose, onSubmit }) => { const [project, setProject] = React.useState(undefined); const [connection, setConnection] = React.useState(undefined); + const modelLocationType = type === ModelLocationType.ObjectStorage ? 'object storage' : 'URI'; return ( { setProject(undefined); setConnection(undefined); @@ -62,7 +66,7 @@ export const ConnectionModal: React.FC<{ /> - Data connection list includes only object storage types that contain a bucket. + {`Data connection list includes only ${modelLocationType} types that contain a bucket.`} diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx index b4541c0fac..eedcd6d37f 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx @@ -15,7 +15,11 @@ import { } from '@patternfly/react-core'; import { OptimizeIcon } from '@patternfly/react-icons'; import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing'; -import { DataConnection, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; +import { + DataConnection, + DataConnectionType, + UpdateObjectAtPropAndValue, +} from '~/pages/projects/types'; import { convertAWSSecretData } from '~/pages/projects/screens/detail/data-connections/utils'; import FormSection from '~/components/pf-overrides/FormSection'; import { ModelVersion } from '~/concepts/modelRegistry/types'; @@ -23,6 +27,8 @@ import { ModelLocationType, RegistrationCommonFormData } from './useRegisterMode import { ConnectionModal } from './ConnectionModal'; import { MR_CHARACTER_LIMIT } from './const'; import { isNameValid } from './utils'; +import { Connection } from '~/concepts/connectionTypes/types'; +import DataConnectionField from '~/pages/projects/screens/spawner/dataConnection/DataConnectionField'; type RegistrationCommonFormSectionsProps = { formData: D; @@ -58,6 +64,10 @@ const RegistrationCommonFormSections = ({ }); }; + const fillURIByConnection = (connection: Connection) => { + setData('modelLocationURI', connection?.data?.URI); + }; + const { versionName, versionDescription, @@ -227,35 +237,57 @@ const RegistrationCommonFormSections = ({ )} - { - setData('modelLocationType', ModelLocationType.URI); - }} - label="URI" - id="location-type-uri" - body={ - modelLocationType === ModelLocationType.URI && ( - - setData('modelLocationURI', value)} - /> - - ) - } - /> + + + { + setData('modelLocationType', ModelLocationType.URI); + }} + label="URI" + id="location-type-uri" + body={ + modelLocationType === ModelLocationType.URI && ( + + setData('modelLocationURI', value)} + /> + + ) + } + /> + + {modelLocationType === ModelLocationType.URI && ( + + + + )} + {isAutofillModalOpen ? ( setAutofillModalOpen(false)} onSubmit={(connection) => { - fillObjectStorageByConnection(connection); + if (modelLocationType === ModelLocationType.ObjectStorage) { + fillObjectStorageByConnection(connection); + } else { + fillURIByConnection(connection); + } setAutofillModalOpen(false); }} /> From aebc3e7a479113484fcc462615848097efaf5b94 Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Tue, 10 Dec 2024 16:56:59 +0530 Subject: [PATCH 02/16] Porting DataConnection to Connection --- .../RegisterModel/ConnectionDropdown.tsx | 23 +++++++++---------- .../screens/RegisterModel/ConnectionModal.tsx | 5 ++-- .../RegistrationCommonFormSections.tsx | 13 +---------- 3 files changed, 14 insertions(+), 27 deletions(-) diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx index b50b6dc1eb..d33bfae3a3 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx @@ -8,15 +8,14 @@ import { Spinner, } from '@patternfly/react-core'; import React from 'react'; -import { DataConnection } from '~/pages/projects/types'; -import { getDataConnectionDisplayName } from '~/pages/projects/screens/detail/data-connections/utils'; -// TODO: temporarily importing across pages so not to interfere with ongoing dataconnection work -import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections'; +import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; type ConnectionDropdownProps = { - onSelect: (connectionInfo: DataConnection) => void; + onSelect: (connectionInfo: Connection) => void; project?: string; - selectedConnection?: DataConnection; + selectedConnection?: Connection; }; export const ConnectionDropdown = ({ onSelect, @@ -24,13 +23,13 @@ export const ConnectionDropdown = ({ selectedConnection, }: ConnectionDropdownProps): React.JSX.Element => { const [isOpen, setIsOpen] = React.useState(false); - const [connections, connectionsLoaded, connectionsLoadError] = useDataConnections(project); + const [connections, connectionsLoaded, connectionsLoadError] = useConnections(project); const onToggle = () => { setIsOpen(!isOpen); }; - const filteredConnections = connections.filter((c) => c.data.data?.AWS_S3_BUCKET); + const filteredConnections = connections.filter((c) => c.data?.AWS_S3_BUCKET); const getToggleContent = () => { if (!project) { @@ -50,7 +49,7 @@ export const ConnectionDropdown = ({ return 'No available data connections'; } if (selectedConnection) { - return getDataConnectionDisplayName(selectedConnection); + return getDisplayNameFromK8sResource(selectedConnection); } return 'Select data connection'; }; @@ -61,7 +60,7 @@ export const ConnectionDropdown = ({ ) => { setIsOpen(false); if (typeof option === 'string') { - const value = connections.find((d) => d.data.metadata.name === option); + const value = connections.find((d) => d.metadata.name === option); if (!value) { return; } @@ -90,8 +89,8 @@ export const ConnectionDropdown = ({ {filteredConnections.map((dataItem) => ( - - {getDataConnectionDisplayName(dataItem)} + + {getDisplayNameFromK8sResource(dataItem)} ))} diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx index 4fc8281f71..8df5ae9959 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { Button, FormGroup, HelperText, Form, FormHelperText } from '@patternfly/react-core'; import { Modal } from '@patternfly/react-core/deprecated'; import ProjectSelector from '~/concepts/projects/ProjectSelector'; -import { DataConnection } from '~/pages/projects/types'; import { Connection } from '~/concepts/connectionTypes/types'; import { ConnectionDropdown } from './ConnectionDropdown'; import { ModelLocationType } from './useRegisterModelData'; @@ -10,10 +9,10 @@ import { ModelLocationType } from './useRegisterModelData'; export const ConnectionModal: React.FC<{ type: ModelLocationType; onClose: () => void; - onSubmit: (connection: DataConnection | Connection) => void; + onSubmit: (connection: Connection) => void; }> = ({ type, onClose, onSubmit }) => { const [project, setProject] = React.useState(undefined); - const [connection, setConnection] = React.useState(undefined); + const [connection, setConnection] = React.useState(undefined); const modelLocationType = type === ModelLocationType.ObjectStorage ? 'object storage' : 'URI'; return ( diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx index eedcd6d37f..ed0b6a3374 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx @@ -15,11 +15,7 @@ import { } from '@patternfly/react-core'; import { OptimizeIcon } from '@patternfly/react-icons'; import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing'; -import { - DataConnection, - DataConnectionType, - UpdateObjectAtPropAndValue, -} from '~/pages/projects/types'; +import { DataConnection, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import { convertAWSSecretData } from '~/pages/projects/screens/detail/data-connections/utils'; import FormSection from '~/components/pf-overrides/FormSection'; import { ModelVersion } from '~/concepts/modelRegistry/types'; @@ -28,7 +24,6 @@ import { ConnectionModal } from './ConnectionModal'; import { MR_CHARACTER_LIMIT } from './const'; import { isNameValid } from './utils'; import { Connection } from '~/concepts/connectionTypes/types'; -import DataConnectionField from '~/pages/projects/screens/spawner/dataConnection/DataConnectionField'; type RegistrationCommonFormSectionsProps = { formData: D; @@ -64,10 +59,6 @@ const RegistrationCommonFormSections = ({ }); }; - const fillURIByConnection = (connection: Connection) => { - setData('modelLocationURI', connection?.data?.URI); - }; - const { versionName, versionDescription, @@ -285,8 +276,6 @@ const RegistrationCommonFormSections = ({ onSubmit={(connection) => { if (modelLocationType === ModelLocationType.ObjectStorage) { fillObjectStorageByConnection(connection); - } else { - fillURIByConnection(connection); } setAutofillModalOpen(false); }} From dae1edfa727f222e20ac9d9a0cd921ff3e01e29f Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Mon, 16 Dec 2024 10:36:34 +0530 Subject: [PATCH 03/16] Merge with 'main' --- .../src/concepts/connectionTypes/utils.ts | 23 +++++ .../RegisterModel/ConnectionDropdown.tsx | 8 +- .../screens/RegisterModel/ConnectionModal.tsx | 1 + .../RegistrationCommonFormSections.tsx | 22 +++-- .../RegisteredModels/useLabeledConnections.ts | 46 +++++++++ ...lDeployModalConnectionFromModelRegistry.ts | 95 +++++++++++++++++++ .../ConnectionSection.tsx | 44 ++++++--- .../ManageInferenceServiceModal.tsx | 13 +++ .../kServeModal/ManageKServeModal.tsx | 16 +++- .../src/pages/modelServing/screens/types.ts | 6 ++ 10 files changed, 252 insertions(+), 22 deletions(-) create mode 100644 frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts create mode 100644 frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index 815997f717..771f0b70c8 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -14,6 +14,8 @@ import { ConnectionTypeValueType, } from '~/concepts/connectionTypes/types'; import { enumIterator } from '~/utilities/utils'; +import { AWSDataEntry } from '~/pages/projects/types'; +import { AwsKeys } from '~/pages/projects/dataConnections/const'; export const isConnectionTypeDataFieldType = ( type: ConnectionTypeFieldTypeUnion | string, @@ -390,3 +392,24 @@ export const validateEnvVarName = (name: string): string | undefined => { } return undefined; }; + +export const convertObjectStorageSecretData = (dataConnection: Connection): AWSDataEntry => { + let convertedData: { key: AwsKeys; value: string }[] = []; + const secretData = dataConnection.data; + if (secretData) { + convertedData = Object.values(AwsKeys) + .filter((key) => key !== AwsKeys.NAME) + .map((key: AwsKeys) => ({ + key, + value: secretData[key] ? window.atob(secretData[key]) : '', + })); + } + const convertedSecret: AWSDataEntry = [ + { + key: AwsKeys.NAME, + value: getDisplayNameFromK8sResource(dataConnection), + }, + ...convertedData, + ]; + return convertedSecret; +}; diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx index d33bfae3a3..3322667cd0 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx @@ -11,13 +11,16 @@ import React from 'react'; import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; import { Connection } from '~/concepts/connectionTypes/types'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; +import { ModelLocationType } from './useRegisterModelData'; type ConnectionDropdownProps = { + type: ModelLocationType; onSelect: (connectionInfo: Connection) => void; project?: string; selectedConnection?: Connection; }; export const ConnectionDropdown = ({ + type, onSelect, project, selectedConnection, @@ -29,7 +32,10 @@ export const ConnectionDropdown = ({ setIsOpen(!isOpen); }; - const filteredConnections = connections.filter((c) => c.data?.AWS_S3_BUCKET); + const filteredConnections = + type === ModelLocationType.ObjectStorage + ? connections.filter((c) => c.data?.AWS_S3_BUCKET) + : connections.filter((c) => c.data?.URI); const getToggleContent = () => { if (!project) { diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx index 8df5ae9959..17da58fe69 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx @@ -59,6 +59,7 @@ export const ConnectionModal: React.FC<{ = { formData: D; @@ -53,12 +53,18 @@ const RegistrationCommonFormSections = ({ AWS_DEFAULT_REGION: 'modelLocationRegion', }; - const fillObjectStorageByConnection = (connection: DataConnection) => { - convertAWSSecretData(connection).forEach((dataItem) => { + const fillObjectStorageByConnection = (connection: Connection) => { + convertObjectStorageSecretData(connection).forEach((dataItem) => { setData(connectionDataMap[dataItem.key], dataItem.value); }); }; + const fillURIByConnection = (connection: Connection) => { + if (connection.data?.URI) { + setData('modelLocationURI', window.atob(connection.data.URI)); + } + }; + const { versionName, versionDescription, @@ -161,7 +167,7 @@ const RegistrationCommonFormSections = ({ icon={} onClick={() => setAutofillModalOpen(true)} > - Autofill from data connection + Autofill from connection )} @@ -263,7 +269,7 @@ const RegistrationCommonFormSections = ({ icon={} onClick={() => setAutofillModalOpen(true)} > - Autofill from data connection + Autofill from connection )} @@ -276,6 +282,8 @@ const RegistrationCommonFormSections = ({ onSubmit={(connection) => { if (modelLocationType === ModelLocationType.ObjectStorage) { fillObjectStorageByConnection(connection); + } else { + fillURIByConnection(connection); } setAutofillModalOpen(false); }} diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts new file mode 100644 index 0000000000..251f0f7299 --- /dev/null +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts @@ -0,0 +1,46 @@ +import React from 'react'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { convertObjectStorageSecretData } from '~/concepts/connectionTypes/utils'; +import { ObjectStorageFields, uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; +import { LabeledConnection } from '~/pages/modelServing/screens/types'; +import { AwsKeys } from '~/pages/projects/dataConnections/const'; + +const useLabeledConnections = ( + modelArtifactUri: string | undefined, + connections: Connection[] = [], +): { + connections: LabeledConnection[]; + storageFields: ObjectStorageFields | null; +} => + React.useMemo(() => { + if (!modelArtifactUri) { + return { + connections: connections.map((connection) => ({ connection })), + storageFields: null, + }; + } + const storageFields = uriToObjectStorageFields(modelArtifactUri); + if (!storageFields) { + return { + connections: connections.map((connection) => ({ connection })), + storageFields, + }; + } + const labeledConnections = connections.map((connection) => { + const awsData = convertObjectStorageSecretData(connection); + const bucket = awsData.find((data) => data.key === AwsKeys.AWS_S3_BUCKET)?.value; + const endpoint = awsData.find((data) => data.key === AwsKeys.S3_ENDPOINT)?.value; + const region = awsData.find((data) => data.key === AwsKeys.DEFAULT_REGION)?.value; + if ( + bucket === storageFields.bucket && + endpoint === storageFields.endpoint && + (region === storageFields.region || !storageFields.region) + ) { + return { connection, isRecommended: true }; + } + return { connection }; + }); + return { connections: labeledConnections, storageFields }; + }, [connections, modelArtifactUri]); + +export default useLabeledConnections; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts new file mode 100644 index 0000000000..01a6415d3d --- /dev/null +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts @@ -0,0 +1,95 @@ +import { AlertVariant } from '@patternfly/react-core'; +import React from 'react'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { ProjectKind } from '~/k8sTypes'; +import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; +import { + CreatingInferenceServiceObject, + InferenceServiceStorageType, + LabeledConnection, +} from '~/pages/modelServing/screens/types'; +import { AwsKeys, EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const'; +import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; +import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; +import useLabeledConnections from './useLabeledConnections'; + +const usePrefillDeployModalConnectionFromModelRegistry = ( + projectContext: { currentProject: ProjectKind; connections: Connection[] } | undefined, + createData: CreatingInferenceServiceObject, + setCreateData: UpdateObjectAtPropAndValue, + registeredModelDeployInfo?: RegisteredModelDeployInfo, +): [LabeledConnection[], boolean, Error | undefined] => { + const [fetchedConnections, connectionsLoaded, connectionsLoadError] = useConnections( + projectContext ? undefined : createData.project, + ); + const allConnections = projectContext?.connections || fetchedConnections; + const { connections, storageFields } = useLabeledConnections( + registeredModelDeployInfo?.modelArtifactUri, + allConnections, + ); + + React.useEffect(() => { + if (registeredModelDeployInfo) { + setCreateData('name', registeredModelDeployInfo.modelName); + const recommendedConnections = connections.filter( + (dataConnection) => dataConnection.isRecommended, + ); + + if (!storageFields) { + setCreateData('storage', { + awsData: EMPTY_AWS_SECRET_DATA, + dataConnection: '', + path: '', + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } else { + const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ + AwsKeys.NAME, + AwsKeys.AWS_S3_BUCKET, + AwsKeys.S3_ENDPOINT, + AwsKeys.DEFAULT_REGION, + ]; + const prefilledAWSData = [ + { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, + { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.bucket }, + { key: AwsKeys.S3_ENDPOINT, value: storageFields.endpoint }, + { key: AwsKeys.DEFAULT_REGION, value: storageFields.region || '' }, + ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), + ]; + if (recommendedConnections.length === 0) { + setCreateData('storage', { + awsData: prefilledAWSData, + dataConnection: '', + path: storageFields.path, + type: InferenceServiceStorageType.NEW_STORAGE, + alert: { + type: AlertVariant.info, + title: + "We've auto-switched to create a new data connection and pre-filled the details for you.", + message: + 'Model location info is available in the registry but no matching data connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', + }, + }); + } else if (recommendedConnections.length === 1) { + setCreateData('storage', { + awsData: prefilledAWSData, + dataConnection: recommendedConnections[0].connection.metadata.name, + path: storageFields.path, + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } else { + setCreateData('storage', { + awsData: prefilledAWSData, + dataConnection: '', + path: storageFields.path, + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } + } + } + }, [connections, storageFields, registeredModelDeployInfo, setCreateData]); + + return [connections, connectionsLoaded, connectionsLoadError]; +}; + +export default usePrefillDeployModalConnectionFromModelRegistry; diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx index 86af4569e2..9516f416fb 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx @@ -1,5 +1,6 @@ import React from 'react'; import { + Alert, Button, Flex, FlexItem, @@ -7,6 +8,7 @@ import { FormSection, Popover, Radio, + Skeleton, Truncate, } from '@patternfly/react-core'; import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; @@ -41,6 +43,7 @@ import TypeaheadSelect, { TypeaheadSelectOption } from '~/components/TypeaheadSe import { CreatingInferenceServiceObject, InferenceServiceStorageType, + LabeledConnection, } from '~/pages/modelServing/screens/types'; import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; @@ -49,7 +52,7 @@ import DataConnectionFolderPathField from './DataConnectionFolderPathField'; type ExistingConnectionFieldProps = { connectionTypes: ConnectionTypeConfigMapObj[]; - projectConnections: Connection[]; + projectConnections: LabeledConnection[]; selectedConnection?: Connection; onSelect: (connection: Connection) => void; folderPath: string; @@ -69,19 +72,19 @@ const ExistingConnectionField: React.FC = ({ const options: TypeaheadSelectOption[] = React.useMemo( () => projectConnections.map((connection) => ({ - content: getDisplayNameFromK8sResource(connection), - value: getResourceNameFromK8sResource(connection), + content: getDisplayNameFromK8sResource(connection.connection), + value: getResourceNameFromK8sResource(connection.connection), description: ( - {getDescriptionFromK8sResource(connection) && ( + {getDescriptionFromK8sResource(connection.connection) && ( - + )} @@ -89,7 +92,7 @@ const ExistingConnectionField: React.FC = ({ ), isSelected: !!selectedConnection && - getResourceNameFromK8sResource(connection) === + getResourceNameFromK8sResource(connection.connection) === getResourceNameFromK8sResource(selectedConnection), })), [connectionTypes, projectConnections, selectedConnection], @@ -129,10 +132,10 @@ const ExistingConnectionField: React.FC = ({ selectOptions={options} onSelect={(_, value) => { const newConnection = projectConnections.find( - (c) => getResourceNameFromK8sResource(c) === value, + (c) => getResourceNameFromK8sResource(c.connection) === value, ); if (newConnection) { - onSelect(newConnection); + onSelect(newConnection.connection); } }} popperProps={{ appendTo: 'inline' }} @@ -271,6 +274,9 @@ type Props = { connection: Connection | undefined; setConnection: (connection?: Connection) => void; setIsConnectionValid: (isValid: boolean) => void; + loaded: boolean; + loadError: Error | undefined; + connections: LabeledConnection[]; }; export const ConnectionSection: React.FC = ({ @@ -279,6 +285,9 @@ export const ConnectionSection: React.FC = ({ connection, setConnection, setIsConnectionValid, + loaded, + loadError, + connections, }) => { const [connectionTypes] = useWatchConnectionTypes(true); const [projectConnections] = useConnections(data.project, true); @@ -299,6 +308,14 @@ export const ConnectionSection: React.FC = ({ } }, [selectedConnection, connection, setConnection]); + if (loadError) { + return ( + + {loadError.message} + + ); + } + return ( <> {data.storage.uri && !hasImagePullSecret && ( @@ -333,10 +350,13 @@ export const ConnectionSection: React.FC = ({ }); }} body={ - data.storage.type === InferenceServiceStorageType.EXISTING_STORAGE && ( + data.storage.type === InferenceServiceStorageType.EXISTING_STORAGE && + (!loaded && data.project !== '' ? ( + + ) : ( { setConnection(selection); @@ -349,7 +369,7 @@ export const ConnectionSection: React.FC = ({ setFolderPath={(path) => setData('storage', { ...data.storage, path })} setIsConnectionValid={setIsConnectionValid} /> - ) + )) } /> ; @@ -56,6 +58,14 @@ const ManageInferenceServiceModal: React.FC = const currentProjectName = projectContext?.currentProject.metadata.name || ''; const currentServingRuntimeName = projectContext?.currentServingRuntime?.metadata.name || ''; + const [connections, connectionsLoaded, connectionsLoadError] = + usePrefillDeployModalConnectionFromModelRegistry( + projectContext, + createData, + setCreateData, + registeredModelDeployInfo, + ); + const [connection, setConnection] = React.useState(); const [isConnectionValid, setIsConnectionValid] = React.useState(false); @@ -182,9 +192,12 @@ const ManageInferenceServiceModal: React.FC = diff --git a/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx b/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx index 49bbaf2597..5c4b9ce68a 100644 --- a/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx +++ b/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx @@ -32,7 +32,7 @@ import { import ServingRuntimeSizeSection from '~/pages/modelServing/screens/projects/ServingRuntimeModal/ServingRuntimeSizeSection'; import ServingRuntimeTemplateSection from '~/pages/modelServing/screens/projects/ServingRuntimeModal/ServingRuntimeTemplateSection'; import ProjectSection from '~/pages/modelServing/screens/projects/InferenceServiceModal/ProjectSection'; -import { DataConnection, NamespaceApplicationCase } from '~/pages/projects/types'; +import { NamespaceApplicationCase } from '~/pages/projects/types'; import InferenceServiceFrameworkSection from '~/pages/modelServing/screens/projects/InferenceServiceModal/InferenceServiceFrameworkSection'; import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils'; import AuthServingRuntimeSection from '~/pages/modelServing/screens/projects/ServingRuntimeModal/AuthServingRuntimeSection'; @@ -54,6 +54,7 @@ import { useProfileIdentifiers } from '~/concepts/hardwareProfiles/utils'; import { useModelServingPodSpecOptionsState } from '~/concepts/hardwareProfiles/useModelServingPodSpecOptionsState'; import { validateEnvVarName } from '~/concepts/connectionTypes/utils'; import { useKServeDeploymentMode } from '~/pages/modelServing/useKServeDeploymentMode'; +import usePrefillDeployModalConnectionFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry'; import KServeAutoscalerReplicaSection from './KServeAutoscalerReplicaSection'; import EnvironmentVariablesSection from './EnvironmentVariablesSection'; import ServingRuntimeArgsSection from './ServingRuntimeArgsSection'; @@ -76,7 +77,7 @@ type ManageKServeModalProps = { { projectContext?: { currentProject: ProjectKind; - dataConnections: DataConnection[]; + connections: Connection[]; }; }, { @@ -136,6 +137,14 @@ const ManageKServeModal: React.FC = ({ podSpecOptionsState.hardwareProfile.formData.selectedProfile, ); + const [connections, connectionsLoaded, connectionsLoadError] = + usePrefillDeployModalConnectionFromModelRegistry( + projectContext, + createDataInferenceService, + setCreateDataInferenceService, + registeredModelDeployInfo, + ); + const [actionInProgress, setActionInProgress] = React.useState(false); const [error, setError] = React.useState(); const [alertVisible, setAlertVisible] = React.useState(true); @@ -405,9 +414,12 @@ const ManageKServeModal: React.FC = ({ )} diff --git a/frontend/src/pages/modelServing/screens/types.ts b/frontend/src/pages/modelServing/screens/types.ts index 2b1addba10..50a981b597 100644 --- a/frontend/src/pages/modelServing/screens/types.ts +++ b/frontend/src/pages/modelServing/screens/types.ts @@ -1,4 +1,5 @@ import { AlertVariant } from '@patternfly/react-core'; +import { Connection } from '~/concepts/connectionTypes/types'; import { ImagePullSecret, SecretKind, ServingContainer, ServingRuntimeKind } from '~/k8sTypes'; import { DataConnection, EnvVariableDataEntry } from '~/pages/projects/types'; import { ContainerResources } from '~/types'; @@ -126,3 +127,8 @@ export type LabeledDataConnection = { dataConnection: DataConnection; isRecommended?: boolean; }; + +export type LabeledConnection = { + connection: Connection; + isRecommended?: boolean; +}; From 6519eda820d3cc4cbfeba25480af068583b17b15 Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Wed, 5 Feb 2025 15:36:46 +0530 Subject: [PATCH 04/16] Fix prefill for S3 connections and merge with main --- frontend/src/components/TypeaheadSelect.tsx | 10 +- .../RegisterModel/ConnectionDropdown.tsx | 2 +- .../RegisterModel/RegisteredModelSelector.tsx | 1 + ...lDeployModalConnectionFromModelRegistry.ts | 7 +- .../components/DeployRegisteredModelModal.tsx | 6 +- .../modelServing/ModelServingContext.tsx | 19 ++- .../screens/global/ServeModelButton.tsx | 3 + .../ConnectionSection.tsx | 125 ++++++++++-------- .../ManageInferenceServiceModal.spec.tsx | 2 + .../ModelMeshSection/ServingRuntimeTable.tsx | 3 + .../screens/projects/ModelServingPlatform.tsx | 3 +- .../overview/serverModels/AddModelFooter.tsx | 3 +- 12 files changed, 118 insertions(+), 66 deletions(-) diff --git a/frontend/src/components/TypeaheadSelect.tsx b/frontend/src/components/TypeaheadSelect.tsx index 57ab4a2978..91ae8c7912 100644 --- a/frontend/src/components/TypeaheadSelect.tsx +++ b/frontend/src/components/TypeaheadSelect.tsx @@ -19,6 +19,8 @@ import { FormHelperText, HelperTextItem, HelperText, + FlexItem, + Flex, } from '@patternfly/react-core'; import { TimesIcon } from '@patternfly/react-icons'; import TruncatedText from '~/components/TruncatedText'; @@ -30,6 +32,7 @@ export interface TypeaheadSelectOption extends Omit { @@ -438,7 +441,7 @@ const TypeaheadSelect: React.FunctionComponent = ({ > {filteredSelections.map((option, index) => { - const { content, value, ...optionProps } = option; + const { content, value, dropdownLabel, ...optionProps } = option; return ( = ({ isFocused={focusedItemIndex === index} {...optionProps} > - {content} + + {content} + {dropdownLabel} + ); })} diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx index 3322667cd0..a62610a110 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx @@ -26,7 +26,7 @@ export const ConnectionDropdown = ({ selectedConnection, }: ConnectionDropdownProps): React.JSX.Element => { const [isOpen, setIsOpen] = React.useState(false); - const [connections, connectionsLoaded, connectionsLoadError] = useConnections(project); + const [connections, connectionsLoaded, connectionsLoadError] = useConnections(project, true); const onToggle = () => { setIsOpen(!isOpen); diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx index d81a865530..225edaf1b5 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegisteredModelSelector.tsx @@ -50,6 +50,7 @@ const RegisteredModelSelector: React.FC = ({ id="model-name" onClearSelection={() => setRegisteredModelId('')} selectOptions={options} + isScrollable placeholder="Select a registered model" noOptionsFoundMessage={(filter) => `No results found for "${filter}"`} onSelect={(_event, selection) => { diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts index 01a6415d3d..b189969a4a 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts @@ -21,6 +21,7 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( ): [LabeledConnection[], boolean, Error | undefined] => { const [fetchedConnections, connectionsLoaded, connectionsLoadError] = useConnections( projectContext ? undefined : createData.project, + true, ); const allConnections = projectContext?.connections || fetchedConnections; const { connections, storageFields } = useLabeledConnections( @@ -29,7 +30,7 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( ); React.useEffect(() => { - if (registeredModelDeployInfo) { + if (registeredModelDeployInfo?.modelArtifactUri) { setCreateData('name', registeredModelDeployInfo.modelName); const recommendedConnections = connections.filter( (dataConnection) => dataConnection.isRecommended, @@ -65,9 +66,9 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( alert: { type: AlertVariant.info, title: - "We've auto-switched to create a new data connection and pre-filled the details for you.", + "We've auto-switched to create a new connection and pre-filled the details for you.", message: - 'Model location info is available in the registry but no matching data connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', + 'Model location info is available in the registry but no matching connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', }, }); } else if (recommendedConnections.length === 1) { diff --git a/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx b/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx index 6803fd951b..21849a7cf2 100644 --- a/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx @@ -19,6 +19,7 @@ import { ModelRegistrySelectorContext } from '~/concepts/modelRegistry/context/M import { getKServeTemplates } from '~/pages/modelServing/customServingRuntimes/utils'; import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections'; import { bumpBothTimestamps } from '~/concepts/modelRegistry/utils/updateTimestamps'; +import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; interface DeployRegisteredModelModalProps { modelVersion: ModelVersion; @@ -48,6 +49,7 @@ const DeployRegisteredModelModal: React.FC = ({ const { loaded: projectDeployStatusLoaded, error: projectError } = useProjectErrorForRegisteredModel(selectedProject?.metadata.name, platform); const [dataConnections] = useDataConnections(selectedProject?.metadata.name); + const [connections] = useConnections(selectedProject?.metadata.name, true); const error = platformError || projectError; const { @@ -143,7 +145,7 @@ const DeployRegisteredModelModal: React.FC = ({ servingRuntimeTemplates={getKServeTemplates(templates, templateOrder, templateDisablement)} shouldFormHidden={!!error} registeredModelDeployInfo={registeredModelDeployInfo} - projectContext={{ currentProject: selectedProject, dataConnections }} + projectContext={{ currentProject: selectedProject, dataConnections, connections }} projectSection={projectSection} /> ); @@ -154,7 +156,7 @@ const DeployRegisteredModelModal: React.FC = ({ onClose={onClose} shouldFormHidden={!!error} registeredModelDeployInfo={registeredModelDeployInfo} - projectContext={{ currentProject: selectedProject, dataConnections }} + projectContext={{ currentProject: selectedProject, dataConnections, connections }} projectSection={projectSection} /> ); diff --git a/frontend/src/pages/modelServing/ModelServingContext.tsx b/frontend/src/pages/modelServing/ModelServingContext.tsx index 4d9fc75bf7..24de481a4c 100644 --- a/frontend/src/pages/modelServing/ModelServingContext.tsx +++ b/frontend/src/pages/modelServing/ModelServingContext.tsx @@ -26,6 +26,8 @@ import { byName, ProjectsContext } from '~/concepts/projects/ProjectsContext'; import { conditionalArea, SupportedArea } from '~/concepts/areas'; import useServingPlatformStatuses from '~/pages/modelServing/useServingPlatformStatuses'; import { useTemplates } from '~/api'; +import { Connection } from '~/concepts/connectionTypes/types'; +import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; import useInferenceServices from './useInferenceServices'; import useServingRuntimes from './useServingRuntimes'; import useTemplateOrder from './customServingRuntimes/useTemplateOrder'; @@ -37,6 +39,7 @@ type ModelServingContextType = { refreshAllData: () => void; filterTokens: (servingRuntime?: string) => SecretKind[]; dataConnections: ContextResourceData; + connections: ContextResourceData; servingRuntimeTemplates: CustomWatchK8sResult; servingRuntimeTemplateOrder: ContextResourceData; servingRuntimeTemplateDisablement: ContextResourceData; @@ -58,6 +61,7 @@ export const ModelServingContext = React.createContext( refreshAllData: () => undefined, filterTokens: () => [], dataConnections: DEFAULT_CONTEXT_DATA, + connections: DEFAULT_CONTEXT_DATA, servingRuntimeTemplates: DEFAULT_LIST_WATCH_RESULT, servingRuntimeTemplateOrder: DEFAULT_CONTEXT_DATA, servingRuntimeTemplateDisablement: DEFAULT_CONTEXT_DATA, @@ -92,15 +96,18 @@ const ModelServingContextProvider = conditionalArea(useDataConnections(namespace)); + const connections = useContextResourceData(useConnections(namespace)); const servingRuntimeRefresh = servingRuntimes.refresh; const inferenceServiceRefresh = inferenceServices.refresh; const dataConnectionRefresh = dataConnections.refresh; + const connectionRefresh = connections.refresh; const refreshAllData = React.useCallback(() => { servingRuntimeRefresh(); inferenceServiceRefresh(); dataConnectionRefresh(); - }, [servingRuntimeRefresh, inferenceServiceRefresh, dataConnectionRefresh]); + connectionRefresh(); + }, [servingRuntimeRefresh, inferenceServiceRefresh, dataConnectionRefresh, connectionRefresh]); const { kServe: { installed: kServeInstalled }, @@ -138,7 +145,8 @@ const ModelServingContextProvider = conditionalArea @@ -165,7 +174,8 @@ const ModelServingContextProvider = conditionalArea - + + + )} void; setIsConnectionValid: (isValid: boolean) => void; - loaded: boolean; - loadError: Error | undefined; - connections: LabeledConnection[]; + loaded?: boolean; + loadError?: Error | undefined; + connections?: LabeledConnection[]; }; export const ConnectionSection: React.FC = ({ @@ -354,21 +373,23 @@ export const ConnectionSection: React.FC = ({ (!loaded && data.project !== '' ? ( ) : ( - { - setConnection(selection); - setData('storage', { - ...data.storage, - dataConnection: getResourceNameFromK8sResource(selection), - }); - }} - folderPath={data.storage.path} - setFolderPath={(path) => setData('storage', { ...data.storage, path })} - setIsConnectionValid={setIsConnectionValid} - /> + connections && ( + { + setConnection(selection); + setData('storage', { + ...data.storage, + dataConnection: getResourceNameFromK8sResource(selection), + }); + }} + folderPath={data.storage.path} + setFolderPath={(path) => setData('storage', { ...data.storage, path })} + setIsConnectionValid={setIsConnectionValid} + /> + ) )) } /> diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/__tests__/ManageInferenceServiceModal.spec.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/__tests__/ManageInferenceServiceModal.spec.tsx index b6a508d25a..60a7003396 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/__tests__/ManageInferenceServiceModal.spec.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/__tests__/ManageInferenceServiceModal.spec.tsx @@ -46,6 +46,7 @@ describe('ManageInferenceServiceModal', () => { projectContext={{ currentProject, dataConnections: [], + connections: [], }} onClose={jest.fn()} />, @@ -69,6 +70,7 @@ describe('ManageInferenceServiceModal', () => { projectContext={{ currentProject: projectChange, dataConnections: [], + connections: [], }} onClose={jest.fn()} />, diff --git a/frontend/src/pages/modelServing/screens/projects/ModelMeshSection/ServingRuntimeTable.tsx b/frontend/src/pages/modelServing/screens/projects/ModelMeshSection/ServingRuntimeTable.tsx index 10dc2d2192..2e7730a436 100644 --- a/frontend/src/pages/modelServing/screens/projects/ModelMeshSection/ServingRuntimeTable.tsx +++ b/frontend/src/pages/modelServing/screens/projects/ModelMeshSection/ServingRuntimeTable.tsx @@ -27,6 +27,7 @@ const ServingRuntimeTable: React.FC = () => { servingRuntimes: { data: modelServers, refresh: refreshServingRuntime }, serverSecrets: { refresh: refreshTokens }, dataConnections: { data: dataConnections, refresh: refreshDataConnections }, + connections: { data: connections, refresh: refreshConnections }, inferenceServices: { data: inferenceServices, refresh: refreshInferenceServices }, filterTokens, currentProject, @@ -99,6 +100,7 @@ const ServingRuntimeTable: React.FC = () => { if (submit) { refreshInferenceServices(); refreshDataConnections(); + refreshConnections(); setExpandedServingRuntimeName(deployServingRuntime.metadata.name); } fireFormTrackingEvent('Model Deployed', { @@ -110,6 +112,7 @@ const ServingRuntimeTable: React.FC = () => { currentProject, currentServingRuntime: deployServingRuntime, dataConnections, + connections, }} /> )} diff --git a/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx b/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx index 566b0b260b..c2f4e9eaac 100644 --- a/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx +++ b/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx @@ -77,6 +77,7 @@ const ModelServingPlatform: React.FC = () => { servingRuntimeTemplateOrder: { data: templateOrder }, servingRuntimeTemplateDisablement: { data: templateDisablement }, dataConnections: { data: dataConnections }, + connections: { data: connections }, serverSecrets: { refresh: refreshTokens }, inferenceServices: { refresh: refreshInferenceServices }, currentProject, @@ -198,7 +199,7 @@ const ModelServingPlatform: React.FC = () => { return ( getTemplateEnabledForPlatform(template, ServingRuntimePlatform.SINGLE), )} diff --git a/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx b/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx index a7ab338496..b7bf820d82 100644 --- a/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx +++ b/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx @@ -33,6 +33,7 @@ const AddModelFooter: React.FC = ({ selectedPlatform, isNIM servingRuntimeTemplateOrder: { data: templateOrder }, servingRuntimeTemplateDisablement: { data: templateDisablement }, dataConnections: { data: dataConnections }, + connections: { data: connections }, serverSecrets: { refresh: refreshTokens }, inferenceServices: { refresh: refreshInferenceServices }, currentProject, @@ -105,7 +106,7 @@ const AddModelFooter: React.FC = ({ selectedPlatform, isNIM ) : null} {modalShown && !isProjectModelMesh && !isNIM ? ( getTemplateEnabledForPlatform(template, ServingRuntimePlatform.SINGLE), )} From eaad6fd147fb3509d1a58f7dda4a2cca10696c4e Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Wed, 5 Feb 2025 15:13:40 +0530 Subject: [PATCH 05/16] Fix prefill for S3 connections --- .../fields/ConnectionTypeFormFields.tsx | 1 + .../src/concepts/connectionTypes/utils.ts | 20 +++++++++++- frontend/src/concepts/modelRegistry/types.ts | 2 ++ .../screens/RegisterModel/utils.ts | 1 + ...lDeployModalConnectionFromModelRegistry.ts | 1 + .../useRegisteredModelDeployInfo.ts | 11 +++++++ .../ConnectionSection.tsx | 31 +++++++++++++++++++ .../src/pages/modelServing/screens/types.ts | 2 ++ 8 files changed, 68 insertions(+), 1 deletion(-) diff --git a/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx b/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx index 8357fec0d7..b7daa64953 100644 --- a/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx +++ b/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx @@ -64,6 +64,7 @@ const ConnectionTypeFormFields: React.FC = ({ return unmatched; }, [connectionValues, fields]); + console.log('connectionValues', connectionValues); const renderDataFields = (dataFields: ConnectionTypeDataField[]) => dataFields.map((field, i) => { const id = `field-${field.envVar}`; diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index 771f0b70c8..e4711432c7 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -14,7 +14,7 @@ import { ConnectionTypeValueType, } from '~/concepts/connectionTypes/types'; import { enumIterator } from '~/utilities/utils'; -import { AWSDataEntry } from '~/pages/projects/types'; +import { AWSDataEntry, EnvVariableDataEntry } from '~/pages/projects/types'; import { AwsKeys } from '~/pages/projects/dataConnections/const'; export const isConnectionTypeDataFieldType = ( @@ -224,6 +224,24 @@ export const getDefaultValues = ( return defaults; }; +export const getMRConnectionValues = ( + connectionValues: EnvVariableDataEntry[] | string, +): { [key: string]: ConnectionTypeValueType } => { + const defaults: { + [key: string]: ConnectionTypeValueType; + } = {}; + if (typeof connectionValues !== 'string') { + connectionValues.map( + (connectionValue) => (defaults[connectionValue.key] = connectionValue.value), + ); + return defaults; + } + if (typeof connectionValues === 'string') { + defaults.URI = connectionValues; + } + return defaults; +}; + export const withRequiredFields = ( connectionType?: ConnectionTypeConfigMapObj, envVars?: string[], diff --git a/frontend/src/concepts/modelRegistry/types.ts b/frontend/src/concepts/modelRegistry/types.ts index 917f9f66bb..af23390d21 100644 --- a/frontend/src/concepts/modelRegistry/types.ts +++ b/frontend/src/concepts/modelRegistry/types.ts @@ -1,4 +1,5 @@ import { K8sAPIOptions } from '~/k8sTypes'; +import { ModelLocationType } from '~/pages/modelRegistry/screens/RegisterModel/useRegisterModelData'; export enum ModelState { LIVE = 'LIVE', @@ -100,6 +101,7 @@ export type ModelArtifact = ModelRegistryBase & { storagePath?: string; modelFormatVersion?: string; serviceAccountName?: string; + modelLocationType?: ModelLocationType; artifactType: string; }; diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/utils.ts b/frontend/src/pages/modelRegistry/screens/RegisterModel/utils.ts index 5767a47859..ea5f6de576 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/utils.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/utils.ts @@ -100,6 +100,7 @@ export const registerVersion = async ( author, modelFormatName: formData.sourceModelFormat, modelFormatVersion: formData.sourceModelFormatVersion, + modelLocationType: formData.modelLocationType, // TODO fill in the name of the data connection we used to prefill if we used one // TODO this should be done as part of https://issues.redhat.com/browse/RHOAIENG-9914 // storageKey: 'TODO', diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts index b189969a4a..f6e2fcc7f7 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts @@ -61,6 +61,7 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( setCreateData('storage', { awsData: prefilledAWSData, dataConnection: '', + connection: registeredModelDeployInfo.modelLocationType, path: storageFields.path, type: InferenceServiceStorageType.NEW_STORAGE, alert: { diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts index e5e3d2b52e..a12eacec5d 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts @@ -2,11 +2,13 @@ import React from 'react'; import useModelArtifactsByVersionId from '~/concepts/modelRegistry/apiHooks/useModelArtifactsByVersionId'; import useRegisteredModelById from '~/concepts/modelRegistry/apiHooks/useRegisteredModelById'; import { ModelVersion } from '~/concepts/modelRegistry/types'; +import { uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; export type RegisteredModelDeployInfo = { modelName: string; modelFormat?: string; modelArtifactUri?: string; + modelLocationType?: string; modelArtifactStorageKey?: string; modelVersionId?: string; registeredModelId?: string; @@ -40,6 +42,14 @@ const useRegisteredModelDeployInfo = ( }; } const modelArtifact = modelArtifactList.items[0]; + const storageFields = uriToObjectStorageFields(modelArtifact.uri || ''); + let modelLocationType; + if (!storageFields) { + modelLocationType = 'uri-v1'; + } + if (storageFields) { + modelLocationType = 's3'; + } return { registeredModelDeployInfo: { modelName, @@ -47,6 +57,7 @@ const useRegisteredModelDeployInfo = ( ? `${modelArtifact.modelFormatName} - ${modelArtifact.modelFormatVersion ?? ''}` : undefined, modelArtifactUri: modelArtifact.uri, + modelLocationType, modelArtifactStorageKey: modelArtifact.storageKey, modelVersionId: modelVersion.id, registeredModelId: modelVersion.registeredModelId, diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx index 4ae3ed93d6..c1f3ef5376 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx @@ -32,6 +32,7 @@ import { getConnectionTypeDisplayName, getConnectionTypeRef, getDefaultValues, + getMRConnectionValues, isConnectionTypeDataField, isUriConnection, isUriConnectionType, @@ -201,12 +202,42 @@ const NewConnectionField: React.FC = ({ ? withRequiredFields(connectionTypes[0], S3ConnectionTypeKeys) : undefined, ); + const { data: nameDescData, onDataChange: setNameDescData } = useK8sNameDescriptionFieldData(); const [connectionValues, setConnectionValues] = React.useState<{ [key: string]: ConnectionTypeValueType; }>(enabledConnectionTypes.length === 1 ? getDefaultValues(enabledConnectionTypes[0]) : {}); + React.useEffect(() => { + const locationType = data.storage.connection; + if (locationType) { + if (locationType === 's3') { + setSelectedConnectionType( + withRequiredFields( + connectionTypes.find((t) => getResourceNameFromK8sResource(t) === locationType), + S3ConnectionTypeKeys, + ), + ); + // setConnectionValues(getMRConnectionValues(data.storage.awsData)); + } + if (data.storage.uri) { + setSelectedConnectionType( + withRequiredFields( + connectionTypes.find( + (t) => getResourceNameFromK8sResource(t) === data.storage.connection, + ), + ['URI'], + ), + ); + setConnectionValues(getMRConnectionValues(data.storage.uri)); + } + } + }, [data.storage.connection, connectionTypes, data.storage.uri, data.storage.awsData]); + + console.log('data.storage.awsData', data.storage.awsData); + console.log('connectionValues', connectionValues); + const [validations, setValidations] = React.useState<{ [key: string]: boolean; }>({}); diff --git a/frontend/src/pages/modelServing/screens/types.ts b/frontend/src/pages/modelServing/screens/types.ts index 50a981b597..199cb747c9 100644 --- a/frontend/src/pages/modelServing/screens/types.ts +++ b/frontend/src/pages/modelServing/screens/types.ts @@ -1,6 +1,7 @@ import { AlertVariant } from '@patternfly/react-core'; import { Connection } from '~/concepts/connectionTypes/types'; import { ImagePullSecret, SecretKind, ServingContainer, ServingRuntimeKind } from '~/k8sTypes'; +import { ModelLocationType } from '~/pages/modelRegistry/screens/RegisterModel/useRegisterModelData'; import { DataConnection, EnvVariableDataEntry } from '~/pages/projects/types'; import { ContainerResources } from '~/types'; @@ -92,6 +93,7 @@ export type InferenceServiceStorage = { type: InferenceServiceStorageType; path: string; dataConnection: string; + connection?: ModelLocationType | string; uri?: string; awsData: EnvVariableDataEntry[]; alert?: { From 99b898ef5688c6061d56bde9dd22322691e72baa Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Wed, 5 Feb 2025 22:41:00 +0530 Subject: [PATCH 06/16] Fix for prefilling URI connections --- .../fields/ConnectionTypeFormFields.tsx | 1 - .../src/concepts/connectionTypes/utils.ts | 9 +- frontend/src/concepts/modelRegistry/utils.ts | 9 +- .../ModelVersionDetailsView.tsx | 20 +- .../usePrefillRegisterVersionFields.ts | 8 +- .../RegisteredModels/useLabeledConnections.ts | 30 +-- .../useLabeledDataConnections.ts | 88 ++++----- ...lDeployModalConnectionFromModelRegistry.ts | 51 ++++- .../usePrefillDeployModalFromModelRegistry.ts | 176 +++++++++--------- .../useRegisteredModelDeployInfo.ts | 4 +- .../components/DeployRegisteredModelModal.tsx | 2 +- .../screens/global/ServeModelButton.tsx | 1 - .../ConnectionSection.tsx | 11 +- .../screens/projects/ModelServingPlatform.tsx | 2 +- .../src/pages/modelServing/screens/types.ts | 2 +- .../overview/serverModels/AddModelFooter.tsx | 2 +- 16 files changed, 231 insertions(+), 185 deletions(-) diff --git a/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx b/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx index b7daa64953..8357fec0d7 100644 --- a/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx +++ b/frontend/src/concepts/connectionTypes/fields/ConnectionTypeFormFields.tsx @@ -64,7 +64,6 @@ const ConnectionTypeFormFields: React.FC = ({ return unmatched; }, [connectionValues, fields]); - console.log('connectionValues', connectionValues); const renderDataFields = (dataFields: ConnectionTypeDataField[]) => dataFields.map((field, i) => { const id = `field-${field.envVar}`; diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index e4711432c7..83b3ff23d2 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -231,9 +231,12 @@ export const getMRConnectionValues = ( [key: string]: ConnectionTypeValueType; } = {}; if (typeof connectionValues !== 'string') { - connectionValues.map( - (connectionValue) => (defaults[connectionValue.key] = connectionValue.value), - ); + connectionValues.map((connectionValue) => { + if (connectionValue.key !== 'Name') { + defaults[connectionValue.key] = connectionValue.value; + } + return defaults; + }); return defaults; } if (typeof connectionValues === 'string') { diff --git a/frontend/src/concepts/modelRegistry/utils.ts b/frontend/src/concepts/modelRegistry/utils.ts index 4d203e93f6..b021533b42 100644 --- a/frontend/src/concepts/modelRegistry/utils.ts +++ b/frontend/src/concepts/modelRegistry/utils.ts @@ -20,7 +20,9 @@ export const objectStorageFieldsToUri = (fields: ObjectStorageFields): string | return `s3://${bucket}/${path}?${searchParams.toString()}`; }; -export const uriToObjectStorageFields = (uri: string): ObjectStorageFields | null => { +export const uriToObjectStorageFields = ( + uri: string, +): { s3Fields: ObjectStorageFields | null; uri: string | null } | null => { try { const urlObj = new URL(uri); // Some environments include the first token after the protocol (our bucket) in the pathname and some have it as the hostname @@ -32,7 +34,10 @@ export const uriToObjectStorageFields = (uri: string): ObjectStorageFields | nul const endpoint = searchParams.get('endpoint'); const region = searchParams.get('defaultRegion'); if (endpoint && bucket && path) { - return { endpoint, bucket, region: region || undefined, path }; + return { s3Fields: { endpoint, bucket, region: region || undefined, path }, uri: null }; + } + if (uri.startsWith('https:')) { + return { s3Fields: null, uri }; } return null; } catch { diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx index 1f4153bc01..4c6d186740 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx @@ -141,51 +141,51 @@ const ModelVersionDetailsView: React.FC = ({ ) : ( <> - {storageFields && ( + {storageFields?.s3Fields && ( <> )} - {!storageFields && ( + {storageFields?.uri && ( <> React.useMemo(() => { if (!modelArtifactUri) { @@ -27,16 +27,24 @@ const useLabeledConnections = ( }; } const labeledConnections = connections.map((connection) => { - const awsData = convertObjectStorageSecretData(connection); - const bucket = awsData.find((data) => data.key === AwsKeys.AWS_S3_BUCKET)?.value; - const endpoint = awsData.find((data) => data.key === AwsKeys.S3_ENDPOINT)?.value; - const region = awsData.find((data) => data.key === AwsKeys.DEFAULT_REGION)?.value; - if ( - bucket === storageFields.bucket && - endpoint === storageFields.endpoint && - (region === storageFields.region || !storageFields.region) - ) { - return { connection, isRecommended: true }; + if (storageFields.s3Fields) { + const awsData = convertObjectStorageSecretData(connection); + const bucket = awsData.find((data) => data.key === AwsKeys.AWS_S3_BUCKET)?.value; + const endpoint = awsData.find((data) => data.key === AwsKeys.S3_ENDPOINT)?.value; + const region = awsData.find((data) => data.key === AwsKeys.DEFAULT_REGION)?.value; + if ( + bucket === storageFields.s3Fields.bucket && + endpoint === storageFields.s3Fields.endpoint && + (region === storageFields.s3Fields.region || !storageFields.s3Fields.region) + ) { + return { connection, isRecommended: true }; + } + } + if (storageFields.uri && connection.data?.URI) { + const findURI = storageFields.uri === window.atob(connection.data.URI); + if (findURI) { + return { connection, isRecommended: true }; + } } return { connection }; }); diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts index e03e7ce376..d41d1c5a59 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts @@ -1,46 +1,46 @@ -import React from 'react'; -import { ObjectStorageFields, uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; -import { LabeledDataConnection } from '~/pages/modelServing/screens/types'; -import { AwsKeys } from '~/pages/projects/dataConnections/const'; -import { convertAWSSecretData } from '~/pages/projects/screens/detail/data-connections/utils'; -import { DataConnection } from '~/pages/projects/types'; +// import React from 'react'; +// import { ObjectStorageFields, uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; +// import { LabeledDataConnection } from '~/pages/modelServing/screens/types'; +// import { AwsKeys } from '~/pages/projects/dataConnections/const'; +// import { convertAWSSecretData } from '~/pages/projects/screens/detail/data-connections/utils'; +// import { DataConnection } from '~/pages/projects/types'; -const useLabeledDataConnections = ( - modelArtifactUri: string | undefined, - dataConnections: DataConnection[] = [], -): { - dataConnections: LabeledDataConnection[]; - storageFields: ObjectStorageFields | null; -} => - React.useMemo(() => { - if (!modelArtifactUri) { - return { - dataConnections: dataConnections.map((dataConnection) => ({ dataConnection })), - storageFields: null, - }; - } - const storageFields = uriToObjectStorageFields(modelArtifactUri); - if (!storageFields) { - return { - dataConnections: dataConnections.map((dataConnection) => ({ dataConnection })), - storageFields, - }; - } - const labeledDataConnections = dataConnections.map((dataConnection) => { - const awsData = convertAWSSecretData(dataConnection); - const bucket = awsData.find((data) => data.key === AwsKeys.AWS_S3_BUCKET)?.value; - const endpoint = awsData.find((data) => data.key === AwsKeys.S3_ENDPOINT)?.value; - const region = awsData.find((data) => data.key === AwsKeys.DEFAULT_REGION)?.value; - if ( - bucket === storageFields.bucket && - endpoint === storageFields.endpoint && - (region === storageFields.region || !storageFields.region) - ) { - return { dataConnection, isRecommended: true }; - } - return { dataConnection }; - }); - return { dataConnections: labeledDataConnections, storageFields }; - }, [dataConnections, modelArtifactUri]); +// const useLabeledDataConnections = ( +// modelArtifactUri: string | undefined, +// dataConnections: DataConnection[] = [], +// ): { +// dataConnections: LabeledDataConnection[]; +// storageFields: ObjectStorageFields | null; +// } => +// React.useMemo(() => { +// if (!modelArtifactUri) { +// return { +// dataConnections: dataConnections.map((dataConnection) => ({ dataConnection })), +// storageFields: null, +// }; +// } +// const storageFields = uriToObjectStorageFields(modelArtifactUri); +// if (!storageFields) { +// return { +// dataConnections: dataConnections.map((dataConnection) => ({ dataConnection })), +// storageFields, +// }; +// } +// const labeledDataConnections = dataConnections.map((dataConnection) => { +// const awsData = convertAWSSecretData(dataConnection); +// const bucket = awsData.find((data) => data.key === AwsKeys.AWS_S3_BUCKET)?.value; +// const endpoint = awsData.find((data) => data.key === AwsKeys.S3_ENDPOINT)?.value; +// const region = awsData.find((data) => data.key === AwsKeys.DEFAULT_REGION)?.value; +// if ( +// bucket === storageFields.bucket && +// endpoint === storageFields.endpoint && +// (region === storageFields.region || !storageFields.region) +// ) { +// return { dataConnection, isRecommended: true }; +// } +// return { dataConnection }; +// }); +// return { dataConnections: labeledDataConnections, storageFields }; +// }, [dataConnections, modelArtifactUri]); -export default useLabeledDataConnections; +// export default useLabeledDataConnections; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts index f6e2fcc7f7..dfe5e2ee96 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts @@ -43,7 +43,7 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( path: '', type: InferenceServiceStorageType.EXISTING_STORAGE, }); - } else { + } else if (storageFields.s3Fields) { const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ AwsKeys.NAME, AwsKeys.AWS_S3_BUCKET, @@ -52,17 +52,17 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( ]; const prefilledAWSData = [ { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, - { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.bucket }, - { key: AwsKeys.S3_ENDPOINT, value: storageFields.endpoint }, - { key: AwsKeys.DEFAULT_REGION, value: storageFields.region || '' }, + { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.s3Fields.bucket }, + { key: AwsKeys.S3_ENDPOINT, value: storageFields.s3Fields.endpoint }, + { key: AwsKeys.DEFAULT_REGION, value: storageFields.s3Fields.region || '' }, ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), ]; if (recommendedConnections.length === 0) { setCreateData('storage', { awsData: prefilledAWSData, dataConnection: '', - connection: registeredModelDeployInfo.modelLocationType, - path: storageFields.path, + connectionType: registeredModelDeployInfo.modelLocationType, + path: storageFields.s3Fields.path, type: InferenceServiceStorageType.NEW_STORAGE, alert: { type: AlertVariant.info, @@ -76,14 +76,49 @@ const usePrefillDeployModalConnectionFromModelRegistry = ( setCreateData('storage', { awsData: prefilledAWSData, dataConnection: recommendedConnections[0].connection.metadata.name, - path: storageFields.path, + path: storageFields.s3Fields.path, type: InferenceServiceStorageType.EXISTING_STORAGE, }); } else { setCreateData('storage', { awsData: prefilledAWSData, dataConnection: '', - path: storageFields.path, + path: storageFields.s3Fields.path, + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } + } else if (storageFields.uri) { + if (recommendedConnections.length === 0) { + setCreateData('storage', { + awsData: EMPTY_AWS_SECRET_DATA, + uri: storageFields.uri, + dataConnection: '', + connectionType: registeredModelDeployInfo.modelLocationType, + path: '', + type: InferenceServiceStorageType.NEW_STORAGE, + alert: { + type: AlertVariant.info, + title: + "We've auto-switched to create a new connection and pre-filled the details for you.", + message: + 'Model location info is available in the registry but no matching connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', + }, + }); + } else if (recommendedConnections.length === 1) { + setCreateData('storage', { + uri: storageFields.uri, + awsData: EMPTY_AWS_SECRET_DATA, + dataConnection: recommendedConnections[0].connection.metadata.name, + path: '', + type: InferenceServiceStorageType.EXISTING_URI, + }); + } else { + setCreateData('storage', { + uri: storageFields.uri, + awsData: EMPTY_AWS_SECRET_DATA, + dataConnection: '', + connectionType: registeredModelDeployInfo.modelLocationType, + path: '', type: InferenceServiceStorageType.EXISTING_STORAGE, }); } diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts index 5adcac3cc9..bfb50e9263 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts @@ -1,93 +1,93 @@ -import { AlertVariant } from '@patternfly/react-core'; -import React from 'react'; -import { ProjectKind } from '~/k8sTypes'; -import useLabeledDataConnections from '~/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections'; -import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; -import { - CreatingInferenceServiceObject, - InferenceServiceStorageType, - LabeledDataConnection, -} from '~/pages/modelServing/screens/types'; -import { AwsKeys, EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const'; -import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections'; -import { DataConnection, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; +// import { AlertVariant } from '@patternfly/react-core'; +// import React from 'react'; +// import { ProjectKind } from '~/k8sTypes'; +// import useLabeledDataConnections from '~/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections'; +// import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; +// import { +// CreatingInferenceServiceObject, +// InferenceServiceStorageType, +// LabeledDataConnection, +// } from '~/pages/modelServing/screens/types'; +// import { AwsKeys, EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const'; +// import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections'; +// import { DataConnection, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; -const usePrefillDeployModalFromModelRegistry = ( - projectContext: { currentProject: ProjectKind; dataConnections: DataConnection[] } | undefined, - createData: CreatingInferenceServiceObject, - setCreateData: UpdateObjectAtPropAndValue, - registeredModelDeployInfo?: RegisteredModelDeployInfo, -): [LabeledDataConnection[], boolean, Error | undefined] => { - const [fetchedDataConnections, dataConnectionsLoaded, dataConnectionsLoadError] = - useDataConnections(projectContext ? undefined : createData.project); - const allDataConnections = projectContext?.dataConnections || fetchedDataConnections; - const { dataConnections, storageFields } = useLabeledDataConnections( - registeredModelDeployInfo?.modelArtifactUri, - allDataConnections, - ); +// const usePrefillDeployModalFromModelRegistry = ( +// projectContext: { currentProject: ProjectKind; dataConnections: DataConnection[] } | undefined, +// createData: CreatingInferenceServiceObject, +// setCreateData: UpdateObjectAtPropAndValue, +// registeredModelDeployInfo?: RegisteredModelDeployInfo, +// ): [LabeledDataConnection[], boolean, Error | undefined] => { +// const [fetchedDataConnections, dataConnectionsLoaded, dataConnectionsLoadError] = +// useDataConnections(projectContext ? undefined : createData.project); +// const allDataConnections = projectContext?.dataConnections || fetchedDataConnections; +// const { dataConnections, storageFields } = useLabeledDataConnections( +// registeredModelDeployInfo?.modelArtifactUri, +// allDataConnections, +// ); - React.useEffect(() => { - if (registeredModelDeployInfo) { - setCreateData('name', registeredModelDeployInfo.modelName); - const recommendedDataConnections = dataConnections.filter( - (dataConnection) => dataConnection.isRecommended, - ); +// React.useEffect(() => { +// if (registeredModelDeployInfo) { +// setCreateData('name', registeredModelDeployInfo.modelName); +// const recommendedDataConnections = dataConnections.filter( +// (dataConnection) => dataConnection.isRecommended, +// ); - if (!storageFields) { - setCreateData('storage', { - awsData: EMPTY_AWS_SECRET_DATA, - dataConnection: '', - path: '', - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } else { - const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ - AwsKeys.NAME, - AwsKeys.AWS_S3_BUCKET, - AwsKeys.S3_ENDPOINT, - AwsKeys.DEFAULT_REGION, - ]; - const prefilledAWSData = [ - { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, - { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.bucket }, - { key: AwsKeys.S3_ENDPOINT, value: storageFields.endpoint }, - { key: AwsKeys.DEFAULT_REGION, value: storageFields.region || '' }, - ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), - ]; - if (recommendedDataConnections.length === 0) { - setCreateData('storage', { - awsData: prefilledAWSData, - dataConnection: '', - path: storageFields.path, - type: InferenceServiceStorageType.NEW_STORAGE, - alert: { - type: AlertVariant.info, - title: - "We've auto-switched to create a new data connection and pre-filled the details for you.", - message: - 'Model location info is available in the registry but no matching data connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', - }, - }); - } else if (recommendedDataConnections.length === 1) { - setCreateData('storage', { - awsData: prefilledAWSData, - dataConnection: recommendedDataConnections[0].dataConnection.data.metadata.name, - path: storageFields.path, - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } else { - setCreateData('storage', { - awsData: prefilledAWSData, - dataConnection: '', - path: storageFields.path, - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } - } - } - }, [dataConnections, storageFields, registeredModelDeployInfo, setCreateData]); +// if (!storageFields) { +// setCreateData('storage', { +// awsData: EMPTY_AWS_SECRET_DATA, +// dataConnection: '', +// path: '', +// type: InferenceServiceStorageType.EXISTING_STORAGE, +// }); +// } else { +// const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ +// AwsKeys.NAME, +// AwsKeys.AWS_S3_BUCKET, +// AwsKeys.S3_ENDPOINT, +// AwsKeys.DEFAULT_REGION, +// ]; +// const prefilledAWSData = [ +// { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, +// { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.bucket }, +// { key: AwsKeys.S3_ENDPOINT, value: storageFields.endpoint }, +// { key: AwsKeys.DEFAULT_REGION, value: storageFields.region || '' }, +// ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), +// ]; +// if (recommendedDataConnections.length === 0) { +// setCreateData('storage', { +// awsData: prefilledAWSData, +// dataConnection: '', +// path: storageFields.path, +// type: InferenceServiceStorageType.NEW_STORAGE, +// alert: { +// type: AlertVariant.info, +// title: +// "We've auto-switched to create a new data connection and pre-filled the details for you.", +// message: +// 'Model location info is available in the registry but no matching data connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', +// }, +// }); +// } else if (recommendedDataConnections.length === 1) { +// setCreateData('storage', { +// awsData: prefilledAWSData, +// dataConnection: recommendedDataConnections[0].dataConnection.data.metadata.name, +// path: storageFields.path, +// type: InferenceServiceStorageType.EXISTING_STORAGE, +// }); +// } else { +// setCreateData('storage', { +// awsData: prefilledAWSData, +// dataConnection: '', +// path: storageFields.path, +// type: InferenceServiceStorageType.EXISTING_STORAGE, +// }); +// } +// } +// } +// }, [dataConnections, storageFields, registeredModelDeployInfo, setCreateData]); - return [dataConnections, dataConnectionsLoaded, dataConnectionsLoadError]; -}; +// return [dataConnections, dataConnectionsLoaded, dataConnectionsLoadError]; +// }; -export default usePrefillDeployModalFromModelRegistry; +// export default usePrefillDeployModalFromModelRegistry; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts index a12eacec5d..c2ba9d09c1 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts @@ -44,10 +44,10 @@ const useRegisteredModelDeployInfo = ( const modelArtifact = modelArtifactList.items[0]; const storageFields = uriToObjectStorageFields(modelArtifact.uri || ''); let modelLocationType; - if (!storageFields) { + if (storageFields?.uri) { modelLocationType = 'uri-v1'; } - if (storageFields) { + if (storageFields?.s3Fields) { modelLocationType = 's3'; } return { diff --git a/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx b/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx index 21849a7cf2..3ae41c8a3c 100644 --- a/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/components/DeployRegisteredModelModal.tsx @@ -145,7 +145,7 @@ const DeployRegisteredModelModal: React.FC = ({ servingRuntimeTemplates={getKServeTemplates(templates, templateOrder, templateDisablement)} shouldFormHidden={!!error} registeredModelDeployInfo={registeredModelDeployInfo} - projectContext={{ currentProject: selectedProject, dataConnections, connections }} + projectContext={{ currentProject: selectedProject, connections }} projectSection={projectSection} /> ); diff --git a/frontend/src/pages/modelServing/screens/global/ServeModelButton.tsx b/frontend/src/pages/modelServing/screens/global/ServeModelButton.tsx index 7ad01004f9..1d8c544091 100644 --- a/frontend/src/pages/modelServing/screens/global/ServeModelButton.tsx +++ b/frontend/src/pages/modelServing/screens/global/ServeModelButton.tsx @@ -107,7 +107,6 @@ const ServeModelButton: React.FC = () => { diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx index c1f3ef5376..65ba0c66a2 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx @@ -210,7 +210,7 @@ const NewConnectionField: React.FC = ({ }>(enabledConnectionTypes.length === 1 ? getDefaultValues(enabledConnectionTypes[0]) : {}); React.useEffect(() => { - const locationType = data.storage.connection; + const locationType = data.storage.connectionType; if (locationType) { if (locationType === 's3') { setSelectedConnectionType( @@ -219,13 +219,13 @@ const NewConnectionField: React.FC = ({ S3ConnectionTypeKeys, ), ); - // setConnectionValues(getMRConnectionValues(data.storage.awsData)); + setConnectionValues(getMRConnectionValues(data.storage.awsData)); } if (data.storage.uri) { setSelectedConnectionType( withRequiredFields( connectionTypes.find( - (t) => getResourceNameFromK8sResource(t) === data.storage.connection, + (t) => getResourceNameFromK8sResource(t) === data.storage.connectionType, ), ['URI'], ), @@ -233,10 +233,7 @@ const NewConnectionField: React.FC = ({ setConnectionValues(getMRConnectionValues(data.storage.uri)); } } - }, [data.storage.connection, connectionTypes, data.storage.uri, data.storage.awsData]); - - console.log('data.storage.awsData', data.storage.awsData); - console.log('connectionValues', connectionValues); + }, [data.storage.connectionType, connectionTypes, data.storage.uri, data.storage.awsData]); const [validations, setValidations] = React.useState<{ [key: string]: boolean; diff --git a/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx b/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx index c2f4e9eaac..3681eb70f4 100644 --- a/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx +++ b/frontend/src/pages/modelServing/screens/projects/ModelServingPlatform.tsx @@ -199,7 +199,7 @@ const ModelServingPlatform: React.FC = () => { return ( getTemplateEnabledForPlatform(template, ServingRuntimePlatform.SINGLE), )} diff --git a/frontend/src/pages/modelServing/screens/types.ts b/frontend/src/pages/modelServing/screens/types.ts index 199cb747c9..002d6633e1 100644 --- a/frontend/src/pages/modelServing/screens/types.ts +++ b/frontend/src/pages/modelServing/screens/types.ts @@ -93,7 +93,7 @@ export type InferenceServiceStorage = { type: InferenceServiceStorageType; path: string; dataConnection: string; - connection?: ModelLocationType | string; + connectionType?: ModelLocationType | string; uri?: string; awsData: EnvVariableDataEntry[]; alert?: { diff --git a/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx b/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx index b7bf820d82..8638fba2af 100644 --- a/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx +++ b/frontend/src/pages/projects/screens/detail/overview/serverModels/AddModelFooter.tsx @@ -106,7 +106,7 @@ const AddModelFooter: React.FC = ({ selectedPlatform, isNIM ) : null} {modalShown && !isProjectModelMesh && !isNIM ? ( getTemplateEnabledForPlatform(template, ServingRuntimePlatform.SINGLE), )} From f70f3c5fd0ba55ecb43161a129771b714bb8cdf1 Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Tue, 11 Feb 2025 20:24:33 +0530 Subject: [PATCH 07/16] Add alert to prefilled modal and some name changes --- .../modelRegistry/__tests__/utils.spec.ts | 50 ++-- ...refillDeployModalFromModelRegistry.spec.ts | 6 +- frontend/src/concepts/modelRegistry/utils.ts | 2 +- .../ModelVersionDetailsView.tsx | 4 +- .../usePrefillRegisterVersionFields.ts | 5 +- .../RegisteredModels/useLabeledConnections.ts | 4 +- .../useLabeledDataConnections.ts | 46 ---- ...lDeployModalConnectionFromModelRegistry.ts | 132 ----------- .../usePrefillDeployModalFromModelRegistry.ts | 215 +++++++++++------- .../useRegisteredModelDeployInfo.ts | 4 +- .../ConnectionSection.tsx | 29 ++- .../ManageInferenceServiceModal.tsx | 4 +- .../kServeModal/ManageKServeModal.tsx | 6 +- 13 files changed, 199 insertions(+), 308 deletions(-) delete mode 100644 frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts delete mode 100644 frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts diff --git a/frontend/src/concepts/modelRegistry/__tests__/utils.spec.ts b/frontend/src/concepts/modelRegistry/__tests__/utils.spec.ts index 95a80e406c..c6724f94de 100644 --- a/frontend/src/concepts/modelRegistry/__tests__/utils.spec.ts +++ b/frontend/src/concepts/modelRegistry/__tests__/utils.spec.ts @@ -6,9 +6,8 @@ import { filterLiveModels, filterLiveVersions, getLastCreatedItem, - ObjectStorageFields, objectStorageFieldsToUri, - uriToObjectStorageFields, + uriToStorageFields, } from '~/concepts/modelRegistry/utils'; import { RegisteredModel, ModelState, ModelVersion } from '~/concepts/modelRegistry/types'; @@ -79,54 +78,67 @@ describe('objectStorageFieldsToUri', () => { }); }); -describe('uriToObjectStorageFields', () => { +describe('uriToStorageFields', () => { it('converts URI to fields with all params present', () => { - const fields = uriToObjectStorageFields( + const fields = uriToStorageFields( 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', ); expect(fields).toEqual({ - endpoint: 'http://s3.amazonaws.com/', - bucket: 'test-bucket', - region: 'us-east-1', - path: 'demo-models/flan-t5-small-caikit', - } satisfies ObjectStorageFields); + s3Fields: { + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + region: 'us-east-1', + path: 'demo-models/flan-t5-small-caikit', + }, + uri: null, + }); }); it('converts URI to fields with region missing', () => { - const fields = uriToObjectStorageFields( + const fields = uriToStorageFields( 's3://test-bucket/demo-models/flan-t5-small-caikit?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F', ); expect(fields).toEqual({ - endpoint: 'http://s3.amazonaws.com/', - bucket: 'test-bucket', - path: 'demo-models/flan-t5-small-caikit', - region: undefined, - } satisfies ObjectStorageFields); + s3Fields: { + endpoint: 'http://s3.amazonaws.com/', + bucket: 'test-bucket', + path: 'demo-models/flan-t5-small-caikit', + region: undefined, + }, + uri: null, + }); }); it('falls back to null if endpoint is missing', () => { - const fields = uriToObjectStorageFields('s3://test-bucket/demo-models/flan-t5-small-caikit'); + const fields = uriToStorageFields('s3://test-bucket/demo-models/flan-t5-small-caikit'); expect(fields).toBeNull(); }); it('falls back to null if path is missing', () => { - const fields = uriToObjectStorageFields( + const fields = uriToStorageFields( 's3://test-bucket/?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', ); expect(fields).toBeNull(); }); it('falls back to null if bucket is missing', () => { - const fields = uriToObjectStorageFields( + const fields = uriToStorageFields( 's3://?endpoint=http%3A%2F%2Fs3.amazonaws.com%2F&defaultRegion=us-east-1', ); expect(fields).toBeNull(); }); it('falls back to null if the URI is malformed', () => { - const fields = uriToObjectStorageFields('test-bucket/demo-models/flan-t5-small-caikit'); + const fields = uriToStorageFields('test-bucket/demo-models/flan-t5-small-caikit'); expect(fields).toBeNull(); }); + it('returns the same URI', () => { + const fields = uriToStorageFields('https://model-repository/folder.zip'); + expect(fields).toEqual({ + s3Fields: null, + uri: 'https://model-repository/folder.zip', + }); + }); }); describe('getLastCreatedItem', () => { diff --git a/frontend/src/concepts/modelRegistry/apiHooks/__tests__/usePrefillDeployModalFromModelRegistry.spec.ts b/frontend/src/concepts/modelRegistry/apiHooks/__tests__/usePrefillDeployModalFromModelRegistry.spec.ts index aaf344654b..c27bf8ed01 100644 --- a/frontend/src/concepts/modelRegistry/apiHooks/__tests__/usePrefillDeployModalFromModelRegistry.spec.ts +++ b/frontend/src/concepts/modelRegistry/apiHooks/__tests__/usePrefillDeployModalFromModelRegistry.spec.ts @@ -1,9 +1,9 @@ import { renderHook } from '@testing-library/react'; -import usePrefillDeployModalFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry'; import { ProjectKind } from '~/k8sTypes'; import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; import { mockInferenceServiceModalData } from '~/__mocks__/mockInferenceServiceModalData'; -import { DataConnection } from '~/pages/projects/types'; +import { Connection } from '~/concepts/connectionTypes/types'; +import usePrefillDeployModalFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry'; describe('usePrefillDeployModalFromModelRegistry', () => { const mockProjectContext = { @@ -15,7 +15,7 @@ describe('usePrefillDeployModalFromModelRegistry', () => { namespace: 'test-namespace', }, } as ProjectKind, - dataConnections: [] as DataConnection[], + connections: [] as Connection[], }; const data = mockInferenceServiceModalData({}); diff --git a/frontend/src/concepts/modelRegistry/utils.ts b/frontend/src/concepts/modelRegistry/utils.ts index b021533b42..5e23ac53e7 100644 --- a/frontend/src/concepts/modelRegistry/utils.ts +++ b/frontend/src/concepts/modelRegistry/utils.ts @@ -20,7 +20,7 @@ export const objectStorageFieldsToUri = (fields: ObjectStorageFields): string | return `s3://${bucket}/${path}?${searchParams.toString()}`; }; -export const uriToObjectStorageFields = ( +export const uriToStorageFields = ( uri: string, ): { s3Fields: ObjectStorageFields | null; uri: string | null } | null => { try { diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx index 4c6d186740..62b8963874 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionDetails/ModelVersionDetailsView.tsx @@ -19,7 +19,7 @@ import { getLabels, mergeUpdatedLabels } from '~/pages/modelRegistry/screens/uti import useModelArtifactsByVersionId from '~/concepts/modelRegistry/apiHooks/useModelArtifactsByVersionId'; import { ModelRegistryContext } from '~/concepts/modelRegistry/context/ModelRegistryContext'; import ModelTimestamp from '~/pages/modelRegistry/screens/components/ModelTimestamp'; -import { uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; +import { uriToStorageFields } from '~/concepts/modelRegistry/utils'; import InlineTruncatedClipboardCopy from '~/components/InlineTruncatedClipboardCopy'; import { bumpBothTimestamps, @@ -42,7 +42,7 @@ const ModelVersionDetailsView: React.FC = ({ const modelArtifact = modelArtifacts.items.length ? modelArtifacts.items[0] : null; const { apiState } = React.useContext(ModelRegistryContext); - const storageFields = uriToObjectStorageFields(modelArtifact?.uri || ''); + const storageFields = uriToStorageFields(modelArtifact?.uri || ''); if (!modelArtifactsLoaded) { return ( diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts b/frontend/src/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts index cf0f53804e..622505f8d7 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/usePrefillRegisterVersionFields.ts @@ -3,7 +3,7 @@ import { RegisteredModel, ModelVersion, ModelArtifact } from '~/concepts/modelRe import { filterLiveVersions, getLastCreatedItem, - uriToObjectStorageFields, + uriToStorageFields, } from '~/concepts/modelRegistry/utils'; import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; import useModelArtifactsByVersionId from '~/concepts/modelRegistry/apiHooks/useModelArtifactsByVersionId'; @@ -53,8 +53,7 @@ export const usePrefillRegisterVersionFields = ({ setData('sourceModelFormat', latestArtifact.modelFormatName || ''); setData('sourceModelFormatVersion', latestArtifact.modelFormatVersion || ''); - const decodedUri = - (latestArtifact.uri && uriToObjectStorageFields(latestArtifact.uri)) || null; + const decodedUri = (latestArtifact.uri && uriToStorageFields(latestArtifact.uri)) || null; setData('modelLocationType', ModelLocationType.ObjectStorage); if (decodedUri?.s3Fields) { diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts index eea1304f63..402cdf2229 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledConnections.ts @@ -1,7 +1,7 @@ import React from 'react'; import { Connection } from '~/concepts/connectionTypes/types'; import { convertObjectStorageSecretData } from '~/concepts/connectionTypes/utils'; -import { ObjectStorageFields, uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; +import { ObjectStorageFields, uriToStorageFields } from '~/concepts/modelRegistry/utils'; import { LabeledConnection } from '~/pages/modelServing/screens/types'; import { AwsKeys } from '~/pages/projects/dataConnections/const'; @@ -19,7 +19,7 @@ const useLabeledConnections = ( storageFields: null, }; } - const storageFields = uriToObjectStorageFields(modelArtifactUri); + const storageFields = uriToStorageFields(modelArtifactUri); if (!storageFields) { return { connections: connections.map((connection) => ({ connection })), diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts deleted file mode 100644 index d41d1c5a59..0000000000 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections.ts +++ /dev/null @@ -1,46 +0,0 @@ -// import React from 'react'; -// import { ObjectStorageFields, uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; -// import { LabeledDataConnection } from '~/pages/modelServing/screens/types'; -// import { AwsKeys } from '~/pages/projects/dataConnections/const'; -// import { convertAWSSecretData } from '~/pages/projects/screens/detail/data-connections/utils'; -// import { DataConnection } from '~/pages/projects/types'; - -// const useLabeledDataConnections = ( -// modelArtifactUri: string | undefined, -// dataConnections: DataConnection[] = [], -// ): { -// dataConnections: LabeledDataConnection[]; -// storageFields: ObjectStorageFields | null; -// } => -// React.useMemo(() => { -// if (!modelArtifactUri) { -// return { -// dataConnections: dataConnections.map((dataConnection) => ({ dataConnection })), -// storageFields: null, -// }; -// } -// const storageFields = uriToObjectStorageFields(modelArtifactUri); -// if (!storageFields) { -// return { -// dataConnections: dataConnections.map((dataConnection) => ({ dataConnection })), -// storageFields, -// }; -// } -// const labeledDataConnections = dataConnections.map((dataConnection) => { -// const awsData = convertAWSSecretData(dataConnection); -// const bucket = awsData.find((data) => data.key === AwsKeys.AWS_S3_BUCKET)?.value; -// const endpoint = awsData.find((data) => data.key === AwsKeys.S3_ENDPOINT)?.value; -// const region = awsData.find((data) => data.key === AwsKeys.DEFAULT_REGION)?.value; -// if ( -// bucket === storageFields.bucket && -// endpoint === storageFields.endpoint && -// (region === storageFields.region || !storageFields.region) -// ) { -// return { dataConnection, isRecommended: true }; -// } -// return { dataConnection }; -// }); -// return { dataConnections: labeledDataConnections, storageFields }; -// }, [dataConnections, modelArtifactUri]); - -// export default useLabeledDataConnections; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts deleted file mode 100644 index dfe5e2ee96..0000000000 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry.ts +++ /dev/null @@ -1,132 +0,0 @@ -import { AlertVariant } from '@patternfly/react-core'; -import React from 'react'; -import { Connection } from '~/concepts/connectionTypes/types'; -import { ProjectKind } from '~/k8sTypes'; -import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; -import { - CreatingInferenceServiceObject, - InferenceServiceStorageType, - LabeledConnection, -} from '~/pages/modelServing/screens/types'; -import { AwsKeys, EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const'; -import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; -import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; -import useLabeledConnections from './useLabeledConnections'; - -const usePrefillDeployModalConnectionFromModelRegistry = ( - projectContext: { currentProject: ProjectKind; connections: Connection[] } | undefined, - createData: CreatingInferenceServiceObject, - setCreateData: UpdateObjectAtPropAndValue, - registeredModelDeployInfo?: RegisteredModelDeployInfo, -): [LabeledConnection[], boolean, Error | undefined] => { - const [fetchedConnections, connectionsLoaded, connectionsLoadError] = useConnections( - projectContext ? undefined : createData.project, - true, - ); - const allConnections = projectContext?.connections || fetchedConnections; - const { connections, storageFields } = useLabeledConnections( - registeredModelDeployInfo?.modelArtifactUri, - allConnections, - ); - - React.useEffect(() => { - if (registeredModelDeployInfo?.modelArtifactUri) { - setCreateData('name', registeredModelDeployInfo.modelName); - const recommendedConnections = connections.filter( - (dataConnection) => dataConnection.isRecommended, - ); - - if (!storageFields) { - setCreateData('storage', { - awsData: EMPTY_AWS_SECRET_DATA, - dataConnection: '', - path: '', - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } else if (storageFields.s3Fields) { - const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ - AwsKeys.NAME, - AwsKeys.AWS_S3_BUCKET, - AwsKeys.S3_ENDPOINT, - AwsKeys.DEFAULT_REGION, - ]; - const prefilledAWSData = [ - { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, - { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.s3Fields.bucket }, - { key: AwsKeys.S3_ENDPOINT, value: storageFields.s3Fields.endpoint }, - { key: AwsKeys.DEFAULT_REGION, value: storageFields.s3Fields.region || '' }, - ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), - ]; - if (recommendedConnections.length === 0) { - setCreateData('storage', { - awsData: prefilledAWSData, - dataConnection: '', - connectionType: registeredModelDeployInfo.modelLocationType, - path: storageFields.s3Fields.path, - type: InferenceServiceStorageType.NEW_STORAGE, - alert: { - type: AlertVariant.info, - title: - "We've auto-switched to create a new connection and pre-filled the details for you.", - message: - 'Model location info is available in the registry but no matching connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', - }, - }); - } else if (recommendedConnections.length === 1) { - setCreateData('storage', { - awsData: prefilledAWSData, - dataConnection: recommendedConnections[0].connection.metadata.name, - path: storageFields.s3Fields.path, - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } else { - setCreateData('storage', { - awsData: prefilledAWSData, - dataConnection: '', - path: storageFields.s3Fields.path, - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } - } else if (storageFields.uri) { - if (recommendedConnections.length === 0) { - setCreateData('storage', { - awsData: EMPTY_AWS_SECRET_DATA, - uri: storageFields.uri, - dataConnection: '', - connectionType: registeredModelDeployInfo.modelLocationType, - path: '', - type: InferenceServiceStorageType.NEW_STORAGE, - alert: { - type: AlertVariant.info, - title: - "We've auto-switched to create a new connection and pre-filled the details for you.", - message: - 'Model location info is available in the registry but no matching connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', - }, - }); - } else if (recommendedConnections.length === 1) { - setCreateData('storage', { - uri: storageFields.uri, - awsData: EMPTY_AWS_SECRET_DATA, - dataConnection: recommendedConnections[0].connection.metadata.name, - path: '', - type: InferenceServiceStorageType.EXISTING_URI, - }); - } else { - setCreateData('storage', { - uri: storageFields.uri, - awsData: EMPTY_AWS_SECRET_DATA, - dataConnection: '', - connectionType: registeredModelDeployInfo.modelLocationType, - path: '', - type: InferenceServiceStorageType.EXISTING_STORAGE, - }); - } - } - } - }, [connections, storageFields, registeredModelDeployInfo, setCreateData]); - - return [connections, connectionsLoaded, connectionsLoadError]; -}; - -export default usePrefillDeployModalConnectionFromModelRegistry; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts index bfb50e9263..17ba02d94b 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry.ts @@ -1,93 +1,132 @@ -// import { AlertVariant } from '@patternfly/react-core'; -// import React from 'react'; -// import { ProjectKind } from '~/k8sTypes'; -// import useLabeledDataConnections from '~/pages/modelRegistry/screens/RegisteredModels/useLabeledDataConnections'; -// import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; -// import { -// CreatingInferenceServiceObject, -// InferenceServiceStorageType, -// LabeledDataConnection, -// } from '~/pages/modelServing/screens/types'; -// import { AwsKeys, EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const'; -// import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections'; -// import { DataConnection, UpdateObjectAtPropAndValue } from '~/pages/projects/types'; +import { AlertVariant } from '@patternfly/react-core'; +import React from 'react'; +import { Connection } from '~/concepts/connectionTypes/types'; +import { ProjectKind } from '~/k8sTypes'; +import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo'; +import { + CreatingInferenceServiceObject, + InferenceServiceStorageType, + LabeledConnection, +} from '~/pages/modelServing/screens/types'; +import { AwsKeys, EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const'; +import { UpdateObjectAtPropAndValue } from '~/pages/projects/types'; +import useConnections from '~/pages/projects/screens/detail/connections/useConnections'; +import useLabeledConnections from './useLabeledConnections'; -// const usePrefillDeployModalFromModelRegistry = ( -// projectContext: { currentProject: ProjectKind; dataConnections: DataConnection[] } | undefined, -// createData: CreatingInferenceServiceObject, -// setCreateData: UpdateObjectAtPropAndValue, -// registeredModelDeployInfo?: RegisteredModelDeployInfo, -// ): [LabeledDataConnection[], boolean, Error | undefined] => { -// const [fetchedDataConnections, dataConnectionsLoaded, dataConnectionsLoadError] = -// useDataConnections(projectContext ? undefined : createData.project); -// const allDataConnections = projectContext?.dataConnections || fetchedDataConnections; -// const { dataConnections, storageFields } = useLabeledDataConnections( -// registeredModelDeployInfo?.modelArtifactUri, -// allDataConnections, -// ); +const usePrefillDeployModalFromModelRegistry = ( + projectContext: { currentProject: ProjectKind; connections: Connection[] } | undefined, + createData: CreatingInferenceServiceObject, + setCreateData: UpdateObjectAtPropAndValue, + registeredModelDeployInfo?: RegisteredModelDeployInfo, +): [LabeledConnection[], boolean, Error | undefined] => { + const [fetchedConnections, connectionsLoaded, connectionsLoadError] = useConnections( + projectContext ? undefined : createData.project, + true, + ); + const allConnections = projectContext?.connections || fetchedConnections; + const { connections, storageFields } = useLabeledConnections( + registeredModelDeployInfo?.modelArtifactUri, + allConnections, + ); -// React.useEffect(() => { -// if (registeredModelDeployInfo) { -// setCreateData('name', registeredModelDeployInfo.modelName); -// const recommendedDataConnections = dataConnections.filter( -// (dataConnection) => dataConnection.isRecommended, -// ); + React.useEffect(() => { + if (registeredModelDeployInfo?.modelArtifactUri) { + setCreateData('name', registeredModelDeployInfo.modelName); + const recommendedConnections = connections.filter( + (dataConnection) => dataConnection.isRecommended, + ); -// if (!storageFields) { -// setCreateData('storage', { -// awsData: EMPTY_AWS_SECRET_DATA, -// dataConnection: '', -// path: '', -// type: InferenceServiceStorageType.EXISTING_STORAGE, -// }); -// } else { -// const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ -// AwsKeys.NAME, -// AwsKeys.AWS_S3_BUCKET, -// AwsKeys.S3_ENDPOINT, -// AwsKeys.DEFAULT_REGION, -// ]; -// const prefilledAWSData = [ -// { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, -// { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.bucket }, -// { key: AwsKeys.S3_ENDPOINT, value: storageFields.endpoint }, -// { key: AwsKeys.DEFAULT_REGION, value: storageFields.region || '' }, -// ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), -// ]; -// if (recommendedDataConnections.length === 0) { -// setCreateData('storage', { -// awsData: prefilledAWSData, -// dataConnection: '', -// path: storageFields.path, -// type: InferenceServiceStorageType.NEW_STORAGE, -// alert: { -// type: AlertVariant.info, -// title: -// "We've auto-switched to create a new data connection and pre-filled the details for you.", -// message: -// 'Model location info is available in the registry but no matching data connection in the project. So we automatically switched the option to create a new data connection and prefilled the information.', -// }, -// }); -// } else if (recommendedDataConnections.length === 1) { -// setCreateData('storage', { -// awsData: prefilledAWSData, -// dataConnection: recommendedDataConnections[0].dataConnection.data.metadata.name, -// path: storageFields.path, -// type: InferenceServiceStorageType.EXISTING_STORAGE, -// }); -// } else { -// setCreateData('storage', { -// awsData: prefilledAWSData, -// dataConnection: '', -// path: storageFields.path, -// type: InferenceServiceStorageType.EXISTING_STORAGE, -// }); -// } -// } -// } -// }, [dataConnections, storageFields, registeredModelDeployInfo, setCreateData]); + if (!storageFields) { + setCreateData('storage', { + awsData: EMPTY_AWS_SECRET_DATA, + dataConnection: '', + path: '', + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } else if (storageFields.s3Fields) { + const prefilledKeys: (typeof EMPTY_AWS_SECRET_DATA)[number]['key'][] = [ + AwsKeys.NAME, + AwsKeys.AWS_S3_BUCKET, + AwsKeys.S3_ENDPOINT, + AwsKeys.DEFAULT_REGION, + ]; + const prefilledAWSData = [ + { key: AwsKeys.NAME, value: registeredModelDeployInfo.modelArtifactStorageKey || '' }, + { key: AwsKeys.AWS_S3_BUCKET, value: storageFields.s3Fields.bucket }, + { key: AwsKeys.S3_ENDPOINT, value: storageFields.s3Fields.endpoint }, + { key: AwsKeys.DEFAULT_REGION, value: storageFields.s3Fields.region || '' }, + ...EMPTY_AWS_SECRET_DATA.filter((item) => !prefilledKeys.includes(item.key)), + ]; + if (recommendedConnections.length === 0) { + setCreateData('storage', { + awsData: prefilledAWSData, + dataConnection: '', + connectionType: registeredModelDeployInfo.modelLocationType, + path: storageFields.s3Fields.path, + type: InferenceServiceStorageType.NEW_STORAGE, + alert: { + type: AlertVariant.info, + title: + "We've auto-switched to create a new connection and pre-filled the details for you.", + message: + 'Model location info is available in the registry but no matching connection in the project. So we automatically switched the option to create a new connection and prefilled the information.', + }, + }); + } else if (recommendedConnections.length === 1) { + setCreateData('storage', { + awsData: prefilledAWSData, + dataConnection: recommendedConnections[0].connection.metadata.name, + path: storageFields.s3Fields.path, + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } else { + setCreateData('storage', { + awsData: prefilledAWSData, + dataConnection: '', + path: storageFields.s3Fields.path, + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } + } else if (storageFields.uri) { + if (recommendedConnections.length === 0) { + setCreateData('storage', { + awsData: EMPTY_AWS_SECRET_DATA, + uri: storageFields.uri, + dataConnection: '', + connectionType: registeredModelDeployInfo.modelLocationType, + path: '', + type: InferenceServiceStorageType.NEW_STORAGE, + alert: { + type: AlertVariant.info, + title: + "We've auto-switched to create a new connection and pre-filled the details for you.", + message: + 'Model location info is available in the registry but no matching connection in the project. So we automatically switched the option to create a new connection and prefilled the information.', + }, + }); + } else if (recommendedConnections.length === 1) { + setCreateData('storage', { + uri: storageFields.uri, + awsData: EMPTY_AWS_SECRET_DATA, + dataConnection: recommendedConnections[0].connection.metadata.name, + path: '', + type: InferenceServiceStorageType.EXISTING_URI, + }); + } else { + setCreateData('storage', { + uri: storageFields.uri, + awsData: EMPTY_AWS_SECRET_DATA, + dataConnection: '', + connectionType: registeredModelDeployInfo.modelLocationType, + path: '', + type: InferenceServiceStorageType.EXISTING_STORAGE, + }); + } + } + } + }, [connections, storageFields, registeredModelDeployInfo, setCreateData]); -// return [dataConnections, dataConnectionsLoaded, dataConnectionsLoadError]; -// }; + return [connections, connectionsLoaded, connectionsLoadError]; +}; -// export default usePrefillDeployModalFromModelRegistry; +export default usePrefillDeployModalFromModelRegistry; diff --git a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts index c2ba9d09c1..bcac59d589 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts +++ b/frontend/src/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo.ts @@ -2,7 +2,7 @@ import React from 'react'; import useModelArtifactsByVersionId from '~/concepts/modelRegistry/apiHooks/useModelArtifactsByVersionId'; import useRegisteredModelById from '~/concepts/modelRegistry/apiHooks/useRegisteredModelById'; import { ModelVersion } from '~/concepts/modelRegistry/types'; -import { uriToObjectStorageFields } from '~/concepts/modelRegistry/utils'; +import { uriToStorageFields } from '~/concepts/modelRegistry/utils'; export type RegisteredModelDeployInfo = { modelName: string; @@ -42,7 +42,7 @@ const useRegisteredModelDeployInfo = ( }; } const modelArtifact = modelArtifactList.items[0]; - const storageFields = uriToObjectStorageFields(modelArtifact.uri || ''); + const storageFields = uriToStorageFields(modelArtifact.uri || ''); let modelLocationType; if (storageFields?.uri) { modelLocationType = 'uri-v1'; diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx index 65ba0c66a2..838e175fcc 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection.tsx @@ -10,6 +10,8 @@ import { Popover, Radio, Skeleton, + Stack, + StackItem, Truncate, } from '@patternfly/react-core'; import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; @@ -438,13 +440,26 @@ export const ConnectionSection: React.FC = ({ }} body={ data.storage.type === InferenceServiceStorageType.NEW_STORAGE && ( - + + {data.storage.alert && ( + + + {data.storage.alert.message} + + + )} + + ) } /> diff --git a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx index a0b5aec8f3..bca874fd03 100644 --- a/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx +++ b/frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx @@ -18,7 +18,7 @@ import K8sNameDescriptionField, { useK8sNameDescriptionFieldData, } from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField'; import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescriptionField/utils'; -import usePrefillDeployModalConnectionFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry'; +import usePrefillDeployModalFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry'; import ProjectSection from './ProjectSection'; import InferenceServiceFrameworkSection from './InferenceServiceFrameworkSection'; import InferenceServiceServingRuntimeSection from './InferenceServiceServingRuntimeSection'; @@ -59,7 +59,7 @@ const ManageInferenceServiceModal: React.FC = const currentServingRuntimeName = projectContext?.currentServingRuntime?.metadata.name || ''; const [connections, connectionsLoaded, connectionsLoadError] = - usePrefillDeployModalConnectionFromModelRegistry( + usePrefillDeployModalFromModelRegistry( projectContext, createData, setCreateData, diff --git a/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx b/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx index 5c4b9ce68a..492e88f88a 100644 --- a/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx +++ b/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx @@ -53,8 +53,12 @@ import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescription import { useProfileIdentifiers } from '~/concepts/hardwareProfiles/utils'; import { useModelServingPodSpecOptionsState } from '~/concepts/hardwareProfiles/useModelServingPodSpecOptionsState'; import { validateEnvVarName } from '~/concepts/connectionTypes/utils'; +<<<<<<< HEAD import { useKServeDeploymentMode } from '~/pages/modelServing/useKServeDeploymentMode'; import usePrefillDeployModalConnectionFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry'; +======= +import usePrefillDeployModalFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry'; +>>>>>>> 6e27303e (Add alert to prefilled modal and some name changes) import KServeAutoscalerReplicaSection from './KServeAutoscalerReplicaSection'; import EnvironmentVariablesSection from './EnvironmentVariablesSection'; import ServingRuntimeArgsSection from './ServingRuntimeArgsSection'; @@ -138,7 +142,7 @@ const ManageKServeModal: React.FC = ({ ); const [connections, connectionsLoaded, connectionsLoadError] = - usePrefillDeployModalConnectionFromModelRegistry( + usePrefillDeployModalFromModelRegistry( projectContext, createDataInferenceService, setCreateDataInferenceService, From ece823be34a071eeb0ebce8b4cdd33c7591cee22 Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Tue, 11 Feb 2025 21:31:20 +0530 Subject: [PATCH 08/16] Fix conflicts --- .../screens/projects/kServeModal/ManageKServeModal.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx b/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx index 492e88f88a..074b3d6189 100644 --- a/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx +++ b/frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx @@ -53,12 +53,8 @@ import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescription import { useProfileIdentifiers } from '~/concepts/hardwareProfiles/utils'; import { useModelServingPodSpecOptionsState } from '~/concepts/hardwareProfiles/useModelServingPodSpecOptionsState'; import { validateEnvVarName } from '~/concepts/connectionTypes/utils'; -<<<<<<< HEAD -import { useKServeDeploymentMode } from '~/pages/modelServing/useKServeDeploymentMode'; -import usePrefillDeployModalConnectionFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalConnectionFromModelRegistry'; -======= import usePrefillDeployModalFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry'; ->>>>>>> 6e27303e (Add alert to prefilled modal and some name changes) +import { useKServeDeploymentMode } from '~/pages/modelServing/useKServeDeploymentMode'; import KServeAutoscalerReplicaSection from './KServeAutoscalerReplicaSection'; import EnvironmentVariablesSection from './EnvironmentVariablesSection'; import ServingRuntimeArgsSection from './ServingRuntimeArgsSection'; From 2ca897c66098c9c2db164fd5dacdcabd0531d5f4 Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Thu, 13 Feb 2025 14:39:57 +0530 Subject: [PATCH 09/16] Add unit tests and UX fixes --- .../mocked/modelRegistry/registerModel.cy.ts | 8 ++--- .../__tests__/useConnectionType.spec.ts | 35 +++++++++++++++++++ frontend/src/concepts/modelRegistry/utils.ts | 5 +-- .../RegisterModel/ConnectionDropdown.tsx | 8 ++--- .../screens/RegisterModel/ConnectionModal.tsx | 10 +++--- .../RegistrationCommonFormSections.tsx | 1 - .../screens/RegisterModel/utils.ts | 1 - .../usePrefillDeployModalFromModelRegistry.ts | 24 +++++-------- .../components/DeployRegisteredModelModal.tsx | 2 +- .../ConnectionSection.tsx | 2 +- .../__tests__/ConnectionSection.spec.tsx | 10 ++++++ 11 files changed, 71 insertions(+), 35 deletions(-) create mode 100644 frontend/src/concepts/connectionTypes/__tests__/useConnectionType.spec.ts diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registerModel.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registerModel.cy.ts index d16d74865d..1cb8c4b48f 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registerModel.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/registerModel.cy.ts @@ -175,9 +175,9 @@ describe('Register model page', () => { registerModelPage.findObjectStorageAutofillButton().click(); registerModelPage .findConnectionSelector() - .contains('Select a project to view its available data connections'); + .contains('Select a project to view its available connections'); registerModelPage.projectDropdown.openAndSelectItem('Test Project', true); - registerModelPage.findConnectionSelector().contains('No available data connections'); + registerModelPage.findConnectionSelector().contains('No available connections'); }); it('Project selection with connections displays connections and fills form', () => { @@ -190,9 +190,9 @@ describe('Register model page', () => { registerModelPage.findObjectStorageAutofillButton().click(); registerModelPage .findConnectionSelector() - .contains('Select a project to view its available data connections'); + .contains('Select a project to view its available connections'); registerModelPage.projectDropdown.openAndSelectItem('Test Project', true); - registerModelPage.findConnectionSelector().contains('Select data connection'); + registerModelPage.findConnectionSelector().contains('Select connection'); registerModelPage.findConnectionSelector().findDropdownItem('Test Secret').click(); registerModelPage.findAutofillButton().click(); registerModelPage.findConnectionAutofillModal().should('not.exist'); diff --git a/frontend/src/concepts/connectionTypes/__tests__/useConnectionType.spec.ts b/frontend/src/concepts/connectionTypes/__tests__/useConnectionType.spec.ts new file mode 100644 index 0000000000..5a98b6ec1b --- /dev/null +++ b/frontend/src/concepts/connectionTypes/__tests__/useConnectionType.spec.ts @@ -0,0 +1,35 @@ +import { act } from 'react'; +import { standardUseFetchState, testHook } from '~/__tests__/unit/testUtils/hooks'; +import { fetchConnectionType } from '~/services/connectionTypesService'; +import { useConnectionType } from '~/concepts/connectionTypes/useConnectionType'; +import { mockConnectionTypeConfigMapObj } from '~/__mocks__/mockConnectionType'; + +jest.mock('~/services/connectionTypesService', () => ({ + fetchConnectionType: jest.fn(), +})); + +const mockFetchConnectionType = jest.mocked(fetchConnectionType); + +describe('useConnectionType', () => { + it('should return connection type config map object when name is given', async () => { + const configMapObjMock = mockConnectionTypeConfigMapObj({ name: 'test-connection-type' }); + mockFetchConnectionType.mockResolvedValue(configMapObjMock); + const renderResult = testHook(useConnectionType)('test-connection-type'); + expect(mockFetchConnectionType).toHaveBeenCalledTimes(1); + expect(renderResult).hookToStrictEqual(standardUseFetchState(undefined, false, undefined)); + expect(renderResult).hookToHaveUpdateCount(1); + + // wait for update + await renderResult.waitForNextUpdate(); + expect(mockFetchConnectionType).toHaveBeenCalledTimes(1); + expect(renderResult).hookToStrictEqual(standardUseFetchState(configMapObjMock, true)); + expect(renderResult).hookToHaveUpdateCount(2); + expect(renderResult).hookToBeStable([false, false, true, true]); + + // refresh + await act(() => renderResult.result.current[3]()); + expect(mockFetchConnectionType).toHaveBeenCalledTimes(2); + expect(renderResult).hookToHaveUpdateCount(3); + expect(renderResult).hookToBeStable([false, true, true, true]); + }); +}); diff --git a/frontend/src/concepts/modelRegistry/utils.ts b/frontend/src/concepts/modelRegistry/utils.ts index 5e23ac53e7..baa0b69fe8 100644 --- a/frontend/src/concepts/modelRegistry/utils.ts +++ b/frontend/src/concepts/modelRegistry/utils.ts @@ -36,10 +36,7 @@ export const uriToStorageFields = ( if (endpoint && bucket && path) { return { s3Fields: { endpoint, bucket, region: region || undefined, path }, uri: null }; } - if (uri.startsWith('https:')) { - return { s3Fields: null, uri }; - } - return null; + return { s3Fields: null, uri }; } catch { return null; } diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx index a62610a110..1691525c0d 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionDropdown.tsx @@ -39,7 +39,7 @@ export const ConnectionDropdown = ({ const getToggleContent = () => { if (!project) { - return 'Select a project to view its available data connections'; + return 'Select a project to view its available connections'; } if (connectionsLoadError) { return 'Error loading connections'; @@ -47,17 +47,17 @@ export const ConnectionDropdown = ({ if (!connectionsLoaded) { return ( <> - Loading Data Connections for the selected project... + Loading Connections for the selected project... ); } if (!filteredConnections.length) { - return 'No available data connections'; + return 'No available connections'; } if (selectedConnection) { return getDisplayNameFromK8sResource(selectedConnection); } - return 'Select data connection'; + return 'Select connection'; }; const onSelectConnection = ( diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx index 17da58fe69..ee533ca918 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/ConnectionModal.tsx @@ -20,8 +20,8 @@ export const ConnectionModal: React.FC<{ isOpen data-testid="connection-autofill-modal" variant="medium" - title="Autofill from data connection" - description={`Select a project to list its ${modelLocationType} data connections. Select a data connection to autofill the model location.`} + title="Autofill from connection" + description={`Select a project to list its ${modelLocationType} connections. Select a connection to autofill the model location.`} onClose={() => { setProject(undefined); setConnection(undefined); @@ -57,7 +57,7 @@ export const ConnectionModal: React.FC<{ invalidDropdownPlaceholder="Select project" /> - + - {`Data connection list includes only ${modelLocationType} types that contain a bucket.`} + {modelLocationType !== 'URI' + ? `Connection list includes only ${modelLocationType} types that contain a bucket.` + : `Connection list includes only ${modelLocationType} types.`} diff --git a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx index 6d0f511b36..a43a8f8711 100644 --- a/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx +++ b/frontend/src/pages/modelRegistry/screens/RegisterModel/RegistrationCommonFormSections.tsx @@ -265,7 +265,6 @@ const RegistrationCommonFormSections = ({ , ,