Skip to content

Conversation

@inknos
Copy link
Collaborator

@inknos inknos commented Nov 24, 2025

Fixes: https://issues.redhat.com/browse/RUN-3716

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?

API: Add GET /libpod/quadlets/{name}

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 24, 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 lsm5 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@github-actions github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Nov 24, 2025
@packit-as-a-service
Copy link

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

@mheon
Copy link
Member

mheon commented Nov 24, 2025

Out of curiousity, unrelated to this PR: Are you planning to do a refactor of the existing GET /quadlets/json as part of this?

@inknos
Copy link
Collaborator Author

inknos commented Nov 24, 2025

Out of curiousity, unrelated to this PR: Are you planning to do a refactor of the existing GET /quadlets/json as part of this?

I am not planning on it, but let me understand better. what kind of refactor are you thinking of?

@mheon
Copy link
Member

mheon commented Nov 24, 2025

I don't think the existing endpoint really needs work, unless you wanted to change it to better match the new endpoints you're writing.

@inknos inknos force-pushed the get-quadlet-api branch 2 times, most recently from df5c1a9 to 52cfa74 Compare November 25, 2025 11:47
// Use QuadletPrint to get the raw file contents
quadletContents, err := containerEngine.QuadletPrint(r.Context(), name)
if err != nil {
utils.Error(w, http.StatusNotFound, fmt.Errorf("no such quadlet: %s: %w", name, err))
Copy link
Member

Choose a reason for hiding this comment

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

is a 404 for any error returned by print correct? 404 should only mean doesn't exist but looking at the code I see an error like is not a supported quadlet file type which doesn't sounds like a 404 to me but rather a 400 for example.

I guess it doesn't matter to match and handing it out specific status codes means we would need to use typed error in quadlet and then match them here with errors.Is/As to differentiate. Maybe that that will get to complicated for no practical gain.

@inknos inknos force-pushed the get-quadlet-api branch 2 times, most recently from 34bc79f to d8ea89a Compare November 26, 2025 15:04
Fixes: https://issues.redhat.com/browse/RUN-3716

Signed-off-by: Nicola Sella <nsella@redhat.com>
@baude
Copy link
Member

baude commented Nov 26, 2025

LGTM, restarted failed tests.

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

Labels

kind/api-change Change to remote API; merits scrutiny release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants