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

Fixes #37822 - Re-raise non-container push repo validation errors #11145

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Sep 16, 2024

What are the changes introduced in this pull request?

Re-raise non-container push name conflict validation errors on repository saves.

Considerations taken when implementing this change?

The earlier patch was catching all validation errors and rescuing silently. Too wide a net resulting in silenced errors on other validations.

What are the testing steps for this pull request?

  1. Create a product and create a repo.
  2. Try creating another repo of same name.
  3. You should see a proper validation error.
  4. Try any other validation errors to make sure error path looks good.

To test that the container push race condition suppression still works, use steps here: #11129 (comment)

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

Code looks fine to me and it works, @qcjames53 did you want to test the container push race condition again? I don't have podman or container stuff setup on my devel box.

Copy link
Contributor

@qcjames53 qcjames53 left a comment

Choose a reason for hiding this comment

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

Hey Samir,

The race condition that used to exist here is still solved. I'm confident in this fix continuing to address that. It looks like other errors are still going to be thrown here; beyond the testing steps, I'm going to lean on Chris' testing of the error handing for additional repo creation errors since he has already ACKed this PR.

@sjha4 sjha4 merged commit 9456160 into Katello:master Sep 26, 2024
27 checks passed
chris1984 pushed a commit that referenced this pull request Nov 4, 2024
chris1984 pushed a commit that referenced this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants