Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Begin data connection clean up #3660

Merged
merged 4 commits into from
Feb 3, 2025
Merged

Begin data connection clean up #3660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3360b50
Data connection clean up
ashley-o0o Jan 21, 2025
cf70705
Clean up some DataConnections code in Cypress tests
ashley-o0o Jan 23, 2025
cd982ab
Remove connectionTypes SupportedArea
ashley-o0o Jan 24, 2025
29e3643
Cypress fixes
ashley-o0o Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions frontend/src/__mocks__/mockDashboardConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export const mockDashboardConfig = ({
disableModelRegistry = false,
disableModelRegistrySecureDB = false,
disableServingRuntimeParams = false,
disableConnectionTypes = true,
disableStorageClasses = false,
disableNotebookController = false,
disableNIMModelServing = true,
Expand Down Expand Up @@ -173,7 +172,6 @@ export const mockDashboardConfig = ({
disableModelRegistry,
disableModelRegistrySecureDB,
disableServingRuntimeParams,
disableConnectionTypes,
disableStorageClasses,
disableNIMModelServing,
},
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/__tests__/cypress/cypress/pages/clusterStorage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,15 @@ class ClusterStorageModal extends Modal {
.findByTestId('cluster-storage-workbench-select');
}

findWorkbenchName(row: number) {
return this.findWorkbenchTable()
.find(`[data-label=Name]`)
.eq(row)
.findByTestId('typeahead-menu-toggle');
}

findWorkbenchSelectValueField(row: number) {
return this.findWorkbenchSelect(row).findByRole('combobox', {
return this.findWorkbenchName(row).findByRole('combobox', {
name: 'Type to filter',
});
}
Expand Down
48 changes: 0 additions & 48 deletions frontend/src/__tests__/cypress/cypress/pages/dataConnection.ts
View file
Open in desktop

This file was deleted.

34 changes: 22 additions & 12 deletions frontend/src/__tests__/cypress/cypress/pages/modelServing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,12 @@ class InferenceServiceModal extends Modal {
return this.find().findByTestId('alt-form-checkbox-auth');
}

findExistingDataConnectionOption() {
return this.find().findByTestId('existing-data-connection-radio');
findNewConnectionOption() {
return this.find().findByTestId('new-connection-radio');
}

findExistingConnectionOption() {
return this.find().findByTestId('existing-connection-radio');
}

findExternalRouteError() {
Expand All @@ -165,20 +169,26 @@ class InferenceServiceModal extends Modal {
return this.find().findByTestId('service-account-form-name');
}

findNewDataConnectionOption() {
return this.find().findByTestId('new-data-connection-radio');
findExistingConnectionSelect() {
return this.find().findByTestId('typeahead-menu-toggle');
}

findExistingConnectionSelect() {
return this.find()
.findByRole('group', { name: 'Source model location' })
.findByRole('button', { name: 'Options menu' });
findExistingConnectionSelectValueField() {
return this.findExistingConnectionSelect().findByRole('combobox', {
name: 'Type to filter',
});
}

selectExistingConnectionSelectOptionByResourceName() {
this.find().findByText('Test Secret').click();
}

findConnectionNameInput() {
return this.find().findByTestId('connection-name-desc-name');
}

selectExistingConnectionSelectOptionByResourceName(name: string) {
this.findExistingConnectionSelect()
.findSelectOptionByTestId(`inference-service-data-connection ${name}`)
.click();
findConnectionFieldInput() {
return this.find().findByTestId('field URI');
}

findLocationNameInput() {
Expand Down
24 changes: 0 additions & 24 deletions frontend/src/__tests__/cypress/cypress/pages/projects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,6 @@ class CreateEditProjectModal extends Modal {
}
}

class DataConnectionRow extends TableRow {
findWorkbenchConnection() {
return this.find().find(`[data-label="Connected workbenches"]`);
}
}

class ProjectDetails {
visit(project: string) {
cy.visitWithLogin(`/projects/${project}`);
Expand All @@ -210,24 +204,10 @@ class ProjectDetails {
cy.testA11y();
}

findSortButton(name: string) {
return this.findDataConnectionTable().find('thead').findByRole('button', { name });
}

findModelServingPlatform(name: string) {
return this.findComponent('model-server').findByTestId(`${name}-serving-platform-card`);
}

private findDataConnectionTable() {
return cy.findByTestId('data-connection-table');
}

getDataConnectionRow(name: string) {
return new DataConnectionRow(() =>
this.findDataConnectionTable().find(`[data-label=Name]`).contains(name).parents('tr'),
);
}

showProjectResourceDetails() {
return cy.findByTestId('resource-name-icon-button').click();
}
Expand Down Expand Up @@ -313,10 +293,6 @@ class ProjectDetails {
return this;
}

findAddDataConnectionButton() {
return cy.findByTestId('add-data-connection-button');
}

private findTable() {
return cy.findByTestId('notebook-image');
}
Expand Down
46 changes: 0 additions & 46 deletions frontend/src/__tests__/cypress/cypress/pages/workbench.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,52 +376,6 @@ class CreateSpawnerPage {
return cy.findByTestId('add-variable-button');
}

findDataConnectionCheckbox() {
return cy.findByTestId('enable-data-connection-checkbox');
}

findNewDataConnectionRadio() {
return cy.findByTestId('new-data-connection-radio');
}

findExistingDataConnectionRadio() {
return cy.findByTestId('existing-data-connection-type-radio');
}

findExistingDataConnectionSelect() {
return cy.findByTestId('existing-data-connection-select');
}

findExistingDataConnectionSelectValueField() {
return this.findExistingDataConnectionSelect().findByRole('combobox', {
name: 'Type to filter',
});
}

findAwsNameInput() {
return cy.findByTestId('field Name');
}

findAwsKeyInput() {
return cy.findByTestId('field AWS_ACCESS_KEY_ID');
}

findAwsSecretKeyInput() {
return cy.findByTestId('field AWS_SECRET_ACCESS_KEY');
}

findEndpointInput() {
return cy.findByTestId('field AWS_S3_ENDPOINT');
}

findRegionInput() {
return cy.findByTestId('field AWS_DEFAULT_REGION');
}

findBucketInput() {
return cy.findByTestId('field AWS_S3_BUCKET');
}

findContainerSizeInput(name: string) {
return cy.findByTestId('container-size-group').contains(name);
}
Expand Down
18 changes: 0 additions & 18 deletions frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/connectionTypes.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,6 @@ it('Connection types should not be available for non product admins', () => {
connectionTypesPage.findNavItem().should('not.exist');
});

it('Connection types should be hidden by feature flag', () => {
asProductAdminUser();

cy.visitWithLogin('/connectionTypes');
connectionTypesPage.shouldReturnNotFound();

cy.interceptOdh(
'GET /api/config',
mockDashboardConfig({
disableConnectionTypes: false,
}),
);

cy.interceptOdh('GET /api/connection-types', []);
connectionTypesPage.visit();
connectionTypesPage.shouldBeEmpty();
});

describe('Connection types', () => {
beforeEach(() => {
asProductAdminUser();
Expand Down
103 changes: 31 additions & 72 deletions frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelVersionDeploy.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { ServingRuntimePlatform } from '~/types';
import { kserveModal } from '~/__tests__/cypress/cypress/pages/modelServing';
import { mockModelArtifact } from '~/__mocks__/mockModelArtifact';
import { mockNimAccount } from '~/__mocks__/mockNimAccount';
import { mockConnectionTypeConfigMap } from '~/__mocks__/mockConnectionType';

const MODEL_REGISTRY_API_VERSION = 'v1alpha3';

Expand Down Expand Up @@ -211,6 +212,22 @@ const initIntercepts = ({
{ namespace: 'opendatahub' },
),
);
cy.interceptOdh('GET /api/connection-types', [
mockConnectionTypeConfigMap({
displayName: 'URI - v1',
name: 'uri-v1',
category: ['existing-category'],
fields: [
{
type: 'uri',
name: 'URI field test',
envVar: 'URI',
required: true,
properties: {},
},
],
}),
]);

cy.interceptK8sList(NIMAccountModel, mockK8sResourceList([mockNimAccount({})]));
};
Expand Down Expand Up @@ -278,12 +295,7 @@ describe('Deploy model version', () => {
modelVersionDeployModal.selectProjectByName('KServe project');

// Validate name input field
kserveModal
.findModelNameInput()
.should(
'contain.value',
`${registeredModelMocked.name} - ${modelVersionMocked2.name}`.slice(0, 63),
);
emilys314 marked this conversation as resolved.
Show resolved Hide resolved
kserveModal.findModelNameInput().should('exist');

// Validate model framework section
kserveModal.findModelFrameworkSelect().should('be.disabled');
Expand All @@ -296,19 +308,12 @@ describe('Deploy model version', () => {
}`,
).should('exist');

// Validate data connection section
cy.findByText(
"We've auto-switched to create a new data connection and pre-filled the details for you.",
).should('exist');
kserveModal.findNewDataConnectionOption().should('be.checked');
kserveModal.findLocationNameInput().should('have.value', modelArtifactMocked.storageKey);
kserveModal.findLocationBucketInput().should('have.value', 'test-bucket');
kserveModal.findLocationRegionInput().should('have.value', 'test-region');
kserveModal.findLocationEndpointInput().should('have.value', 'test-endpoint');
kserveModal.findLocationPathInput().should('have.value', 'demo-models/test-path');
emilys314 marked this conversation as resolved.
Show resolved Hide resolved
// Validate connection section
kserveModal.findExistingConnectionOption().should('be.checked');
kserveModal.findLocationPathInput().should('exist');
});

it('One match data connection on KServe modal', () => {
it('One match connection on KServe modal', () => {
initIntercepts({});
cy.interceptK8sList(
SecretModel,
Expand All @@ -335,60 +340,14 @@ describe('Deploy model version', () => {
modelVersionRow.findKebabAction('Deploy').click();
modelVersionDeployModal.selectProjectByName('KServe project');

// Validate data connection section
kserveModal.findExistingDataConnectionOption().should('be.checked');
kserveModal.findExistingConnectionSelect().should('contain.text', 'Test SecretRecommended');
kserveModal.findLocationPathInput().should('have.value', 'demo-models/test-path');
});

it('More than one match data connections on KServe modal', () => {
ashley-o0o marked this conversation as resolved.
Show resolved Hide resolved
initIntercepts({});
cy.interceptK8sList(
SecretModel,
mockK8sResourceList([
mockSecretK8sResource({
namespace: 'kserve-project',
s3Bucket: 'dGVzdC1idWNrZXQ=',
endPoint: 'dGVzdC1lbmRwb2ludA==',
region: 'dGVzdC1yZWdpb24=',
}),
mockSecretK8sResource({
name: 'test-secret-2',
displayName: 'Test Secret 2',
namespace: 'kserve-project',
s3Bucket: 'dGVzdC1idWNrZXQ=',
endPoint: 'dGVzdC1lbmRwb2ludA==',
region: 'dGVzdC1yZWdpb24=',
}),
mockSecretK8sResource({
name: 'test-secret-not-match',
displayName: 'Test Secret Not Match',
namespace: 'kserve-project',
s3Bucket: 'dGVzdC1idWNrZXQ=',
endPoint: 'dGVzdC1lbmRwb2ludC1ub3QtbWF0Y2g=', // endpoint not match
region: 'dGVzdC1yZWdpb24=',
}),
]),
);

cy.visit(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`);
const modelVersionRow = modelRegistry.getModelVersionRow('test model version');
modelVersionRow.findKebabAction('Deploy').click();
modelVersionDeployModal.selectProjectByName('KServe project');

// Validate data connection section
kserveModal.findExistingDataConnectionOption().should('be.checked');
kserveModal.findExistingConnectionSelect().should('contain.text', 'Select...');
kserveModal.findLocationPathInput().should('have.value', 'demo-models/test-path');

// Make sure recommended label is there
kserveModal.selectExistingConnectionSelectOptionByResourceName('test-secret');
kserveModal.findExistingConnectionSelect().should('contain.text', 'Recommended');

kserveModal.selectExistingConnectionSelectOptionByResourceName('test-secret-2');
kserveModal.findExistingConnectionSelect().should('contain.text', 'Recommended');

kserveModal.selectExistingConnectionSelectOptionByResourceName('test-secret-not-match');
kserveModal.findExistingConnectionSelect().should('not.contain.text', 'Recommended');
// Validate connection section
kserveModal.findExistingConnectionOption().should('be.checked');
kserveModal.findExistingConnectionSelectValueField().click();
kserveModal.selectExistingConnectionSelectOptionByResourceName();
kserveModal.findLocationPathInput().type('test-model/');
kserveModal.findNewConnectionOption().click();
kserveModal.findExistingConnectionSelect().should('have.attr', 'disabled');
kserveModal.findConnectionNameInput().type('Test Name');
kserveModal.findConnectionFieldInput().type('https://test');
});
});
Loading
Loading