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

fix(backend): Fixes response status of http error code when uploading duplicate pipeline [Fixes #10311] #10546

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

champon1020
Copy link
Contributor

@champon1020 champon1020 commented Mar 7, 2024

Description of your changes:
This is the backend fix of #10311.
I propose following solution but feel free to recommend a better one.

When uploading pipeline which already exists, it returns HTTP 500 error (internal server error) while it should return 409 error (conflict error). In order to solve this problem, I insert a branch by error code using util.IsUserErrorCodeMatch.

Checklist:

Copy link

Hi @champon1020. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Validate the error code of pipeline creation in order to return
the status conflict when the error represents AlreadyExists.

Signed-off-by: champon1020 <[email protected]>
@rimolive
Copy link
Member

rimolive commented Mar 7, 2024

Thank you for your contribution, @champon1020!

/ok-to-test

@rimolive
Copy link
Member

/lgtm

After uploading the same pipeline twice I got this expected error from API:

{"error_message":"Failed to create a pipeline and a pipeline version. The pipeline already exists.: Failed to create a pipeline and a pipeline version: Already exist error: Failed to create a new pipeline. The name some_name already exists. Please specify a new name","error_details":"Failed to create a pipeline and a pipeline version. The pipeline already exists.: Failed to create a pipeline and a pipeline version: Already exist error: Failed to create a new pipeline. The name some_name already exists. Please specify a new name"}

@rimolive
Copy link
Member

@chensun @Tomcli I confirm this PR is working. Please review/approve.

Copy link
Member

@Tomcli Tomcli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Copy link
Member

@Tomcli Tomcli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Tomcli

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rimolive
Copy link
Member

/test kubeflow-pipeline-e2e-test

@google-oss-prow google-oss-prow bot merged commit 96eb87c into kubeflow:master Mar 12, 2024
6 checks passed
petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Mar 27, 2024
… duplicate pipeline [Fixes  kubeflow#10311] (kubeflow#10546)

Validate the error code of pipeline creation in order to return
the status conflict when the error represents AlreadyExists.

Signed-off-by: champon1020 <[email protected]>
petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Mar 29, 2024
… duplicate pipeline [Fixes  kubeflow#10311] (kubeflow#10546)

Validate the error code of pipeline creation in order to return
the status conflict when the error represents AlreadyExists.

Signed-off-by: champon1020 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants