Skip to content

Conversation

@inknos
Copy link
Collaborator

@inknos inknos commented Dec 9, 2025

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

Add DELETE /libpod/quadlets/{name}

waiting on changes of #27659

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 9, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 9, 2025

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.

Details

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-sigs/prow repository.

@openshift-ci openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Dec 9, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 9, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: inknos
Once this PR has been reviewed and has the lgtm label, please assign tomsweeneyredhat for approval. For more information see the Code Review Process.

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

Details 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

@github-actions github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Dec 9, 2025
@inknos inknos force-pushed the delete-quadlet-api branch from aa7f9b1 to 1602f59 Compare December 16, 2025 18:44
@packit-as-a-service
Copy link

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

@inknos inknos force-pushed the delete-quadlet-api branch from 1602f59 to 6f93a7a Compare December 17, 2025 15:45
@inknos inknos marked this pull request as ready for review December 17, 2025 15:46
@inknos inknos force-pushed the delete-quadlet-api branch 7 times, most recently from e1aee32 to 3944b47 Compare December 18, 2025 14:51
Fixes: https://issues.redhat.com/browse/RUN-3742

Signed-off-by: Nicola Sella <nsella@redhat.com>
@inknos inknos force-pushed the delete-quadlet-api branch from 3944b47 to b55a3b7 Compare December 18, 2025 15:21
return
}
// If force=false and quadlet is running, return 400
if !query.Force && strings.Contains(err.Error(), "running") {
Copy link
Member

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.

Copy link
Collaborator Author

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) {
Copy link
Member

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.

Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/api-change Change to remote API; merits scrutiny

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants