Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Bug]: Pipelines server creation fails on incorrectly formatted s3 endpoint #1350

Closed
1 task done
Gkrumbach07 opened this issue Jun 8, 2023 · 6 comments · Fixed by #1778
Closed
1 task done

[Bug]: Pipelines server creation fails on incorrectly formatted s3 endpoint #1350

Gkrumbach07 opened this issue Jun 8, 2023 · 6 comments · Fixed by #1778
Assignees
Labels
feature/ds-pipelines Data Science Pipelines feature (aka DSP) field-priority Flag to track improvements that are for stability -- effort to put in front of new functionality kind/bug Something isn't working priority/normal An issue with the product; fix when possible rhods-1.33

Comments

@Gkrumbach07
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

If you try to create a pipelines server and you configure the aws s3 endpoint to be a url without a scheme, the frontend will throw an error and not create the pipeline server

Expected Behavior

I expect the s3 endpoint to validate if there is a scheme present as it is required in the pipeline server spec.

i.e.
https://s3.amazonaws.com/
❌ s3.amazonaws.com/

Or at least give a better error message instead of Cannot read properties of undefined (reading 'replace')

Steps To Reproduce

Create a pipeline server with a object storage AWS_S3_ENDPOINT field set to s3.amazonaws.com/

Workaround (if any)

Include a scheme to the url

What browsers are you seeing the problem on?

Chrome

Open Data Hub Version

2.11

Anything else

No response

@Gkrumbach07 Gkrumbach07 added kind/bug Something isn't working untriaged Indicates the newly create issue has not been triaged yet feature/ds-pipelines Data Science Pipelines feature (aka DSP) labels Jun 8, 2023
@lucferbux lucferbux added priority/normal An issue with the product; fix when possible and removed untriaged Indicates the newly create issue has not been triaged yet labels Jun 12, 2023
@andrewballantyne andrewballantyne added the field-priority Flag to track improvements that are for stability -- effort to put in front of new functionality label Jun 22, 2023
@manaswinidas
Copy link
Contributor

Possible solutions:

  1. Validation warning while entering data in the S3 endpoint field, something like "Please enter a valid URL"(https://www.patternfly.org/v4/components/text-area#validated) or with additional label info(https://www.patternfly.org/v4/components/form/#form-group-with-additional-label-info)
Screenshot 2023-07-04 at 2 41 25 PM
  1. Have a placeholder text with https:// instead of actual text
  2. Append https:// silently on our side if not present already(without any warning to the user)
  3. https:// or http:// dropdown beside the URL text box
Screenshot 2023-07-04 at 3 00 40 PM

@andrewballantyne
Copy link
Member

andrewballantyne commented Jul 6, 2023

cc @kywalker-rh @yannnz need a UX call here

@guimou @erwangranger would you say s3.amazonaws.com is a valid endpoint? Is it wise to support? (instead of http/https schemas)

@HumairAK What is expected values of schema in the DSPA config?

      externalStorage: {
        bucket: string;
        host: string;
        port?: '';
        scheme: string; // <<< we say this is required, because we determine this from the leading characters
        s3CredentialsSecret: {
          accessKey: string;
          secretKey: string;
          secretName: string;
        };

@strangiato
Copy link
Contributor

+1 to this issue. Another issue is that once you attempt to create the DS Pipeline and receive the above error message, if you created the connection as part of the pipeline creation, you can't edit the pipeline. It gives you an error that the connection already exists. If you switch over to the existing connection, the drop down doesn't include it because the connection didn't exist when the modal was opened.

@HumairAK
Copy link

HumairAK commented Jul 10, 2023

@HumairAK What is expected values of schema in the DSPA config?

Assuming you mean scheme here, possible values are http and https, I have created a task for dsp team to add more meaningful descriptions for the crd fields within the crd spec.

@andrewballantyne
Copy link
Member

@HumairAK I did mean that -- didn't self check the value I was talking about. Thanks for catching that & the issue you linked to 👍

@andrewballantyne
Copy link
Member

@strangiato Thanks for that additional information. We are planning to move away from the Data Connection being directly "related" to the DSPA (Pipeline Server) creation in #1214. This is because the DSPA doesn't react to the changes so it gives the user a false sense of connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/ds-pipelines Data Science Pipelines feature (aka DSP) field-priority Flag to track improvements that are for stability -- effort to put in front of new functionality kind/bug Something isn't working priority/normal An issue with the product; fix when possible rhods-1.33
Projects
Archived in project
Status: Dashboard
7 participants