diff --git a/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts b/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts index 14f1c108c3..532cfcd907 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/connectionTypes.ts @@ -107,6 +107,14 @@ class CreateConnectionTypePage { findDuplicateConnectionTypeButton() { return cy.findByTestId('duplicate-connection-type'); } + + findCompatibleModelServingTypesAlert() { + return cy.findByTestId('compatible-model-serving-types-alert'); + } + + findModelServingCompatibleTypeDropdown() { + return cy.findByTestId('select-model-serving-compatible-type'); + } } class CategorySection extends Contextual { diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts index 3e27a319fc..cca087dc4c 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/createConnectionType.cy.ts @@ -18,20 +18,31 @@ describe('create', () => { disableConnectionTypes: false, }), ); + cy.interceptOdh('GET /api/connection-types', [ + mockConnectionTypeConfigMap({ + displayName: 'URI - v1', + name: 'uri-v1', + fields: [ + { + type: 'uri', + name: 'URI field test', + envVar: 'URI', + required: true, + properties: {}, + }, + ], + }), + ]); }); - it('Display base page', () => { + it('Can create connection type', () => { + const categorySection = createConnectionTypePage.getCategorySection(); createConnectionTypePage.visitCreatePage(); createConnectionTypePage.findConnectionTypeName().should('exist'); createConnectionTypePage.findConnectionTypeDesc().should('exist'); createConnectionTypePage.findConnectionTypeEnableCheckbox().should('exist'); createConnectionTypePage.findConnectionTypePreviewToggle().should('exist'); - }); - - it('Allows create button with valid name and category', () => { - const categorySection = createConnectionTypePage.getCategorySection(); - createConnectionTypePage.visitCreatePage(); createConnectionTypePage.findConnectionTypeName().should('have.value', ''); createConnectionTypePage.findSubmitButton().should('be.disabled'); @@ -40,6 +51,28 @@ describe('create', () => { categorySection.findCategoryTable(); categorySection.findMultiGroupSelectButton('Object-storage'); createConnectionTypePage.findSubmitButton().should('be.enabled'); + + categorySection.findMultiGroupInput().type('Database'); + categorySection.findMultiGroupSelectButton('Database'); + + categorySection.findMultiGroupInput().type('New category'); + + categorySection.findMultiGroupSelectButton('Option'); + categorySection.findChipItem('New category').should('exist'); + categorySection.findMultiGroupInput().type('{esc}'); + + createConnectionTypePage + .findModelServingCompatibleTypeDropdown() + .findDropdownItem('URI') + .click(); + createConnectionTypePage.findCompatibleModelServingTypesAlert().should('exist'); + createConnectionTypePage.getFieldsTableRow(0).findSectionHeading().should('exist'); + createConnectionTypePage + .getFieldsTableRow(1) + .findName() + .should('contain.text', 'URI field test'); + + createConnectionTypePage.findSubmitButton().should('be.enabled'); }); it('Shows creation error message when creation fails', () => { @@ -60,28 +93,6 @@ describe('create', () => { createConnectionTypePage.findFooterError().should('contain.text', 'returned error message'); }); - - it('Selects category or creates new category', () => { - createConnectionTypePage.visitCreatePage(); - - const categorySection = createConnectionTypePage.getCategorySection(); - - categorySection.findCategoryTable(); - categorySection.findMultiGroupSelectButton('Object-storage'); - - categorySection.findChipItem('Object storage').should('exist'); - categorySection.clearMultiChipItem(); - - categorySection.findMultiGroupSelectButton('Object-storage'); - - categorySection.findMultiGroupInput().type('Database'); - categorySection.findMultiGroupSelectButton('Database'); - - categorySection.findMultiGroupInput().type('New category'); - - categorySection.findMultiGroupSelectButton('Option'); - categorySection.findChipItem('New category').should('exist'); - }); }); describe('duplicate', () => { diff --git a/frontend/src/components/SimpleMenuActions.tsx b/frontend/src/components/SimpleMenuActions.tsx index 3fe847e428..46d1ffb53a 100644 --- a/frontend/src/components/SimpleMenuActions.tsx +++ b/frontend/src/components/SimpleMenuActions.tsx @@ -22,12 +22,20 @@ const isSpacer = (v: Item | Spacer): v is Spacer => 'isSpacer' in v; type SimpleDropdownProps = { dropdownItems: (Item | Spacer)[]; testId?: string; + toggleLabel?: string; + variant?: React.ComponentProps['variant']; } & Omit< React.ComponentProps, 'isOpen' | 'isPlain' | 'popperProps' | 'toggle' | 'onChange' >; -const SimpleMenuActions: React.FC = ({ dropdownItems, testId, ...props }) => { +const SimpleMenuActions: React.FC = ({ + dropdownItems, + testId, + toggleLabel, + variant, + ...props +}) => { const [open, setOpen] = React.useState(false); return ( @@ -37,17 +45,17 @@ const SimpleMenuActions: React.FC = ({ dropdownItems, testI onOpenChange={(isOpened) => setOpen(isOpened)} toggle={(toggleRef) => ( setOpen(!open)} isExpanded={open} > - + {toggleLabel ?? } )} - popperProps={{ position: 'right' }} + popperProps={!toggleLabel ? { position: 'right' } : undefined} > {dropdownItems.map((itemOrSpacer, i) => diff --git a/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts b/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts index 824f7ef701..8bbcd78d25 100644 --- a/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts +++ b/frontend/src/concepts/connectionTypes/__tests__/utils.spec.ts @@ -12,7 +12,9 @@ import { fieldTypeToString, getCompatibleTypes, isModelServingCompatible, + isModelServingTypeCompatible, isValidEnvVar, + ModelServingCompatibleConnectionTypes, toConnectionTypeConfigMap, toConnectionTypeConfigMapObj, } from '~/concepts/connectionTypes/utils'; @@ -312,3 +314,29 @@ describe('getCompatibleTypes', () => { ).toEqual([CompatibleTypes.ModelServing]); }); }); + +describe('isModelServingTypeCompatible', () => { + it('should identify model serving compatible env vars', () => { + expect( + isModelServingTypeCompatible(['invalid'], ModelServingCompatibleConnectionTypes.ModelServing), + ).toBe(false); + expect( + isModelServingTypeCompatible( + ['AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY', 'AWS_S3_ENDPOINT', 'AWS_S3_BUCKET'], + ModelServingCompatibleConnectionTypes.ModelServing, + ), + ).toBe(true); + expect( + isModelServingTypeCompatible(['invalid'], ModelServingCompatibleConnectionTypes.OCI), + ).toBe(false); + expect(isModelServingTypeCompatible(['URI'], ModelServingCompatibleConnectionTypes.OCI)).toBe( + true, + ); + expect( + isModelServingTypeCompatible(['invalid'], ModelServingCompatibleConnectionTypes.URI), + ).toBe(false); + expect(isModelServingTypeCompatible(['URI'], ModelServingCompatibleConnectionTypes.URI)).toBe( + true, + ); + }); +}); diff --git a/frontend/src/concepts/connectionTypes/utils.ts b/frontend/src/concepts/connectionTypes/utils.ts index 7acf828ef0..a6578aba1f 100644 --- a/frontend/src/concepts/connectionTypes/utils.ts +++ b/frontend/src/concepts/connectionTypes/utils.ts @@ -117,6 +117,38 @@ export const S3ConnectionTypeKeys = [ 'AWS_S3_BUCKET', ]; +export enum ModelServingCompatibleConnectionTypes { + ModelServing = 's3', + OCI = 'oci-compliant-registry-v1', + URI = 'uri-v1', +} + +const modelServinConnectionTypes: Record< + ModelServingCompatibleConnectionTypes, + { + name: string; + envVars: string[]; + } +> = { + [ModelServingCompatibleConnectionTypes.ModelServing]: { + name: 'S3 compatible object storage', + envVars: S3ConnectionTypeKeys, + }, + [ModelServingCompatibleConnectionTypes.OCI]: { + name: 'OCI compliant registry', + envVars: ['URI'], + }, + [ModelServingCompatibleConnectionTypes.URI]: { + name: 'URI', + envVars: ['URI'], + }, +}; + +export const isModelServingTypeCompatible = ( + envVars: string[], + type: ModelServingCompatibleConnectionTypes, +): boolean => modelServinConnectionTypes[type].envVars.every((envVar) => envVars.includes(envVar)); + export const isModelServingCompatible = (envVars: string[]): boolean => S3ConnectionTypeKeys.every((envVar) => envVars.includes(envVar)); @@ -279,3 +311,19 @@ export const findSectionFields = ( return fields.slice(sectionIndex + 1, nextSectionIndex === -1 ? undefined : nextSectionIndex); }; + +export const filterModelServingCompatibleTypes = ( + connectionTypes: ConnectionTypeConfigMapObj[], +): { + key: ModelServingCompatibleConnectionTypes; + name: string; + type: ConnectionTypeConfigMapObj; +}[] => + enumIterator(ModelServingCompatibleConnectionTypes) + .map(([, value]) => { + const connectionType = connectionTypes.find((t) => t.metadata.name === value); + return connectionType + ? { key: value, name: modelServinConnectionTypes[value].name, type: connectionType } + : undefined; + }) + .filter((t) => t != null); diff --git a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx index 6f609d8ecf..10beaad782 100644 --- a/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx +++ b/frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx @@ -4,13 +4,20 @@ import { AlertActionLink, Button, Checkbox, + Flex, + FlexItem, Form, FormGroup, FormSection, PageSection, + Popover, + Stack, + StackItem, + Text, + TextContent, } from '@patternfly/react-core'; import { useNavigate } from 'react-router'; -import { OpenDrawerRightIcon } from '@patternfly/react-icons'; +import { OpenDrawerRightIcon, OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; import { useUser } from '~/redux/selectors'; import { ConnectionTypeConfigMapObj, @@ -34,6 +41,15 @@ import { connectionTypeFormSchema, ValidationErrorCodes, } from '~/concepts/connectionTypes/validationUtils'; +import { useWatchConnectionTypes } from '~/utilities/useWatchConnectionTypes'; +import { + filterModelServingCompatibleTypes, + isConnectionTypeDataField, + isModelServingTypeCompatible, +} from '~/concepts/connectionTypes/utils'; +import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton'; +import SimpleMenuActions from '~/components/SimpleMenuActions'; +import { joinWithCommaAnd } from '~/utilities/string'; import CreateConnectionTypeFooter from './ManageConnectionTypeFooter'; import ManageConnectionTypeFieldsTable from './ManageConnectionTypeFieldsTable'; import ManageConnectionTypeBreadcrumbs from './ManageConnectionTypeBreadcrumbs'; @@ -48,6 +64,12 @@ const ManageConnectionTypePage: React.FC = ({ prefill, isEdit, onSave }) const navigate = useNavigate(); const { username: currentUsername } = useUser(); + const [connectionTypes] = useWatchConnectionTypes(); + const modelServingCompatibleTypes = React.useMemo( + () => filterModelServingCompatibleTypes(connectionTypes), + [connectionTypes], + ); + const [isDrawerExpanded, setIsDrawerExpanded] = React.useState(false); const initialValues = React.useMemo(() => { @@ -100,6 +122,11 @@ const ManageConnectionTypePage: React.FC = ({ prefill, isEdit, onSave }) [connectionNameDesc, enabled, fields, username, category], ); + const matchedModelServingCompatibleTypes = React.useMemo(() => { + const envVars = fields.filter(isConnectionTypeDataField).map((f) => f.envVar); + return modelServingCompatibleTypes.filter((t) => isModelServingTypeCompatible(envVars, t.key)); + }, [fields, modelServingCompatibleTypes]); + const isValid = React.useMemo(() => isK8sNameDescriptionDataValid(connectionNameDesc), [connectionNameDesc]) && validation.validationResult.success; @@ -200,17 +227,94 @@ const ManageConnectionTypePage: React.FC = ({ prefill, isEdit, onSave }) onChange={(_e, value) => setData('enabled', value)} /> - + + Fields + {modelServingCompatibleTypes.length > 0 ? ( + <> + + ({ + key: t.key, + label: t.name, + onClick: () => { + setData('fields', [ + ...fields, + { + type: 'section', + name: t.name, + description: `Configure the fields that make this connection compatible with ${t.name}.`, + }, + ...(t.type.data?.fields ?? []), + ]); + }, + }))} + /> + + + + + Model serving compatible connection types can be used for model + serving. A connection can use one of multiple different methods, + such as S3 compatible storage or URI, for serving models. + + + Select a type to automatically add the fields required to use its + corresponding model serving method. + + + } + > + } + aria-label="More info for section heading" + /> + + + + ) : undefined} + + } + > - {validation.hasValidationIssue( - ['fields'], - ValidationErrorCodes.FIELDS_ENV_VAR_CONFLICT, - ) ? ( - - Two or more fields are using the same environment variable. Ensure that each - field uses a unique environment variable to proceed. - - ) : null} + + {matchedModelServingCompatibleTypes.length > 0 ? ( + + t.name), + { + singlePrefix: 'the ', + singleSuffix: ' model serving type.', + multiSuffix: ' model serving types.', + }, + )}`} + /> + + ) : undefined} + {validation.hasValidationIssue( + ['fields'], + ValidationErrorCodes.FIELDS_ENV_VAR_CONFLICT, + ) ? ( + + + Two or more fields are using the same environment variable. Ensure that + each field uses a unique environment variable to proceed. + + + ) : undefined} + setData('fields', value)} diff --git a/frontend/src/utilities/__tests__/string.spec.ts b/frontend/src/utilities/__tests__/string.spec.ts index a38d761916..aa007f52e8 100644 --- a/frontend/src/utilities/__tests__/string.spec.ts +++ b/frontend/src/utilities/__tests__/string.spec.ts @@ -7,6 +7,7 @@ import { replaceNumericPartWithString, containsMultipleSlashesPattern, triggerFileDownload, + joinWithCommaAnd, } from '~/utilities/string'; global.URL.createObjectURL = jest.fn(() => 'test-url'); @@ -234,3 +235,35 @@ describe('triggerFileDownload', () => { removeChildSpy.mockRestore(); }); }); + +describe('joinWithCommaAnd', () => { + it('should join items with comma and oxford comma', () => { + expect(joinWithCommaAnd(['item1', 'item2', 'item3'])).toBe('item1, item2, and item3'); + }); + + it('should join items with no comma if only two items', () => { + expect(joinWithCommaAnd(['item1', 'item2'])).toBe('item1 and item2'); + }); + + it('should join items with comma and with custom prefix and suffix', () => { + expect( + joinWithCommaAnd(['item1', 'item2', 'item3'], { + singlePrefix: 'invalid', + singleSuffix: 'invalid', + multiPrefix: 'Prefix ', + multiSuffix: ' suffix.', + }), + ).toBe('Prefix item1, item2, and item3 suffix.'); + }); + + it('should join single item with custom prefix and suffix', () => { + expect( + joinWithCommaAnd(['item1'], { + singlePrefix: 'Prefix ', + singleSuffix: ' suffix.', + multiPrefix: 'invalid', + multiSuffix: 'invalid', + }), + ).toBe('Prefix item1 suffix.'); + }); +}); diff --git a/frontend/src/utilities/string.ts b/frontend/src/utilities/string.ts index 28efc854a0..793d94ad3b 100644 --- a/frontend/src/utilities/string.ts +++ b/frontend/src/utilities/string.ts @@ -113,3 +113,21 @@ export const truncateString = (str: string, length: number): string => { } return `${str.substring(0, length)}…`; }; + +export const joinWithCommaAnd = ( + items: string[], + options?: { + singlePrefix?: string; + singleSuffix?: string; + multiPrefix?: string; + multiSuffix?: string; + }, +): string => + items.length > 1 + ? `${options?.multiPrefix ?? ''}${items + .slice(0, items.length - 1) + .map((i) => i) + .join(', ')}${items.length > 2 ? ',' : ''} and ${items[items.length - 1]}${ + options?.multiSuffix ?? '' + }` + : `${options?.singlePrefix ?? ''}${items[0]}${options?.singleSuffix ?? ''}`;