-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add DELETE /libpod/quadlets #27730
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
base: main
Are you sure you want to change the base?
Add DELETE /libpod/quadlets #27730
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions 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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: inknos The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
aa7f9b1 to
1602f59
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
1602f59 to
6f93a7a
Compare
e1aee32 to
3944b47
Compare
Fixes: https://issues.redhat.com/browse/RUN-3742 Signed-off-by: Nicola Sella <nsella@redhat.com>
3944b47 to
b55a3b7
Compare
| return | ||
| } | ||
| // If force=false and quadlet is running, return 400 | ||
| if !query.Force && strings.Contains(err.Error(), "running") { |
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.
Is it possible to check the error type instead of matching a string? If I just check if the message contains running, it might trigger a false positive if a file is named running.
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.
to check for an error I think the quadlet logic should be changed, and didn't want to touch the libpod function. I can think of a better way to match against it and avoid false positives
| } | ||
|
|
||
| // RemoveQuadlets handles DELETE /libpod/quadlets to remove quadlet files (batch operation) | ||
| func RemoveQuadlets(w http.ResponseWriter, r *http.Request) { |
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.
It looks like RemoveQuatlets and RemoveQuadlet are redundant; we should consider consolidating them.
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.
reason why id didn't is to make a call to DELETE /libpod/quadlets/{name} + args and DELETE /libpod/quadlets + args, and I didn't like to handle the logic of name / no name in the function. I could do it, although this to me seemed more RESTful
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?
waiting on changes of #27659