Skip to content

Commit

Permalink
add model serving compatible select to connection type form
Browse files Browse the repository at this point in the history
  • Loading branch information
christianvogt committed Oct 29, 2024
1 parent 4842b35 commit 80266ff
Show file tree
Hide file tree
Showing 7 changed files with 253 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLElement> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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 - v1')
.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', () => {
Expand All @@ -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', () => {
Expand Down
18 changes: 13 additions & 5 deletions frontend/src/components/SimpleMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof MenuToggle>['variant'];
} & Omit<
React.ComponentProps<typeof Dropdown>,
'isOpen' | 'isPlain' | 'popperProps' | 'toggle' | 'onChange'
>;

const SimpleMenuActions: React.FC<SimpleDropdownProps> = ({ dropdownItems, testId, ...props }) => {
const SimpleMenuActions: React.FC<SimpleDropdownProps> = ({
dropdownItems,
testId,
toggleLabel,
variant,
...props
}) => {
const [open, setOpen] = React.useState(false);

return (
Expand All @@ -37,17 +45,17 @@ const SimpleMenuActions: React.FC<SimpleDropdownProps> = ({ dropdownItems, testI
onOpenChange={(isOpened) => setOpen(isOpened)}
toggle={(toggleRef) => (
<MenuToggle
aria-label="Actions"
aria-label={toggleLabel ?? 'Actions'}
data-testid={testId}
variant="plain"
variant={variant ?? (toggleLabel ? 'default' : 'plain')}
ref={toggleRef}
onClick={() => setOpen(!open)}
isExpanded={open}
>
<EllipsisVIcon />
{toggleLabel ?? <EllipsisVIcon />}
</MenuToggle>
)}
popperProps={{ position: 'right' }}
popperProps={!toggleLabel ? { position: 'right' } : undefined}
>
<DropdownList>
{dropdownItems.map((itemOrSpacer, i) =>
Expand Down
21 changes: 21 additions & 0 deletions frontend/src/concepts/connectionTypes/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,3 +279,24 @@ export const findSectionFields = (

return fields.slice(sectionIndex + 1, nextSectionIndex === -1 ? undefined : nextSectionIndex);
};

const MODEL_SERVING_PREFILL_NAMES = ['s3', 'oci-compliant-registry-v1', 'uri-v1'];

export const filterModelServingPrefillConnectionTypes = (
connectionTypes: ConnectionTypeConfigMapObj[],
): ConnectionTypeConfigMapObj[] =>
connectionTypes.filter((type) => MODEL_SERVING_PREFILL_NAMES.includes(type.metadata.name));

export const isCompatibleWithConnectionType = (
fields: ConnectionTypeField[],
connectionType: ConnectionTypeConfigMapObj,
): boolean => {
const compatibleDataFields = connectionType.data?.fields?.filter(isConnectionTypeDataField);
const dataFields = fields.filter(isConnectionTypeDataField);
if (compatibleDataFields) {
return compatibleDataFields
.filter(isConnectionTypeDataField)
.every((cf) => dataFields.find((f) => cf.envVar === f.envVar));
}
return false;
};
132 changes: 121 additions & 11 deletions frontend/src/pages/connectionTypes/manage/ManageConnectionTypePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -34,6 +41,15 @@ import {
connectionTypeFormSchema,
ValidationErrorCodes,
} from '~/concepts/connectionTypes/validationUtils';
import { useWatchConnectionTypes } from '~/utilities/useWatchConnectionTypes';
import {
filterModelServingPrefillConnectionTypes,
isCompatibleWithConnectionType,
} from '~/concepts/connectionTypes/utils';
import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton';
import SimpleMenuActions from '~/components/SimpleMenuActions';
import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
import { joinWithCommaAnd } from '~/utilities/string';
import CreateConnectionTypeFooter from './ManageConnectionTypeFooter';
import ManageConnectionTypeFieldsTable from './ManageConnectionTypeFieldsTable';
import ManageConnectionTypeBreadcrumbs from './ManageConnectionTypeBreadcrumbs';
Expand All @@ -48,6 +64,15 @@ const ManageConnectionTypePage: React.FC<Props> = ({ prefill, isEdit, onSave })
const navigate = useNavigate();
const { username: currentUsername } = useUser();

const [connectionTypes] = useWatchConnectionTypes();
const modelServingPrefillConnectionTypes = React.useMemo(
() =>
filterModelServingPrefillConnectionTypes(connectionTypes).toSorted((a, b) =>
getDisplayNameFromK8sResource(a).localeCompare(getDisplayNameFromK8sResource(b)),
),
[connectionTypes],
);

const [isDrawerExpanded, setIsDrawerExpanded] = React.useState(false);

const initialValues = React.useMemo<ConnectionTypeFormData>(() => {
Expand Down Expand Up @@ -100,6 +125,12 @@ const ManageConnectionTypePage: React.FC<Props> = ({ prefill, isEdit, onSave })
[connectionNameDesc, enabled, fields, username, category],
);

const compatibleTypes = React.useMemo(
() =>
modelServingPrefillConnectionTypes.filter((t) => isCompatibleWithConnectionType(fields, t)),
[fields, modelServingPrefillConnectionTypes],
);

const isValid =
React.useMemo(() => isK8sNameDescriptionDataValid(connectionNameDesc), [connectionNameDesc]) &&
validation.validationResult.success;
Expand Down Expand Up @@ -200,17 +231,96 @@ const ManageConnectionTypePage: React.FC<Props> = ({ prefill, isEdit, onSave })
onChange={(_e, value) => setData('enabled', value)}
/>
</FormGroup>
<FormSection title="Fields" className="pf-v5-u-mt-0">
<FormSection
className="pf-v5-u-mt-0"
title={
<Flex gap={{ default: 'gapSm' }}>
<FlexItem>Fields</FlexItem>
{modelServingPrefillConnectionTypes.length > 0 ? (
<>
<FlexItem>
<SimpleMenuActions
testId="select-model-serving-compatible-type"
toggleLabel="Select a model serving compatible type"
variant="secondary"
dropdownItems={modelServingPrefillConnectionTypes.map((i) => ({
key: i.metadata.name,
label: getDisplayNameFromK8sResource(i),
onClick: () => {
setData('fields', [
...fields,
{
type: 'section',
name: getDisplayNameFromK8sResource(i),
description: `Configure the fields that make this connection compatible with ${getDisplayNameFromK8sResource(
i,
)}.`,
},
...(i.data?.fields ?? []),
]);
},
}))}
/>
</FlexItem>
<FlexItem>
<Popover
headerContent="Model serving compatible types "
bodyContent={
<TextContent>
<Text>
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.
</Text>
<Text>
Select a type to automatically add the fields required to use its
corresponding model serving method.
</Text>
</TextContent>
}
>
<DashboardPopupIconButton
icon={<OutlinedQuestionCircleIcon />}
aria-label="More info for section heading"
/>
</Popover>
</FlexItem>
</>
) : undefined}
</Flex>
}
>
<FormGroup>
{validation.hasValidationIssue(
['fields'],
ValidationErrorCodes.FIELDS_ENV_VAR_CONFLICT,
) ? (
<Alert isInline variant="danger" title="Environment variables conflict">
Two or more fields are using the same environment variable. Ensure that each
field uses a unique environment variable to proceed.
</Alert>
) : null}
<Stack hasGutter>
{compatibleTypes.length > 0 ? (
<StackItem>
<Alert
data-testid="compatible-model-serving-types-alert"
isInline
variant="info"
title={`This connection type is compatible with ${joinWithCommaAnd(
compatibleTypes.map(getDisplayNameFromK8sResource),
{
singlePrefix: 'the ',
singleSuffix: ' model serving type.',
multiSuffix: ' model serving types.',
},
)}`}
/>
</StackItem>
) : undefined}
{validation.hasValidationIssue(
['fields'],
ValidationErrorCodes.FIELDS_ENV_VAR_CONFLICT,
) ? (
<StackItem>
<Alert isInline variant="danger" title="Environment variables conflict">
Two or more fields are using the same environment variable. Ensure that
each field uses a unique environment variable to proceed.
</Alert>
</StackItem>
) : undefined}
</Stack>
<ManageConnectionTypeFieldsTable
fields={fields}
onFieldsChange={(value) => setData('fields', value)}
Expand Down
33 changes: 33 additions & 0 deletions frontend/src/utilities/__tests__/string.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
replaceNumericPartWithString,
containsMultipleSlashesPattern,
triggerFileDownload,
joinWithCommaAnd,
} from '~/utilities/string';

global.URL.createObjectURL = jest.fn(() => 'test-url');
Expand Down Expand Up @@ -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 comma and no comma if only two items', () => {
expect(joinWithCommaAnd(['item1', 'item2'])).toBe('item1 and item2');
});

it('should join items with comma and with custom prefixes and suffixes', () => {
expect(
joinWithCommaAnd(['item1', 'item2', 'item3'], {
singlePrefix: 'invalid',
singleSuffix: 'invalid',
multiPrefix: 'Prefix ',
multiSuffix: ' suffix.',
}),
).toBe('Prefix item1, item2, and item3 suffix.');
});

it('should join single with custom prefixes and suffixes', () => {
expect(
joinWithCommaAnd(['item1'], {
singlePrefix: 'Prefix ',
singleSuffix: ' suffix.',
multiPrefix: 'invalid',
multiSuffix: 'invalid',
}),
).toBe('Prefix item1 suffix.');
});
});
Loading

0 comments on commit 80266ff

Please sign in to comment.