-
-
Notifications
You must be signed in to change notification settings - Fork 374
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 returncode when deleting a mirror with snapshot #1201
Fix returncode when deleting a mirror with snapshot #1201
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1201 +/- ##
==========================================
- Coverage 65.99% 65.97% -0.02%
==========================================
Files 143 143
Lines 16196 16190 -6
==========================================
- Hits 10688 10682 -6
- Misses 4754 4756 +2
+ Partials 754 752 -2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
It's HTTP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change 404 to 403 in the commit and PR description? Otherwise LGTM
When trying to delete a mirror that has snapshot and not providing the force option, the API should not return a `500 StatusInternalServerError`. A `403 StatusForbidden` is more appropriate when the condition is expected by the server.
1689ee5
to
5e83a20
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
When trying to delete a mirror that has snapshot and not providing the force option, the API should not return a
500 StatusInternalServerError
.A
403 StatusForbidden
is more appropriate when the condition is expected by the server.Fixes #
Requirements
All new code should be covered with tests, documentation should be updated. CI should pass.
I was able to run
make modules install
andmake test
with success but I'm not able to runmake system-test
because of missing dependencies (gpg1).Description of the Change
When implementing a client for Aptly API the
500
HTTP status code returned does not make it clear that there is an issue with the request.From https://www.rfc-editor.org/rfc/rfc9110.html#name-500-internal-server-error
The condition preventing the request to be fulfill is not unexpected and is even documented in the response.
Checklist
- [ ] unit-test added (if change is algorithm)- [ ] functional test added/updated (if change is functional)- [ ] man page updated (if applicable)- [ ] bash completion updated (if applicable)- [ ] documentation updatedAUTHORS