-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[Compat API] Also print successfully tagging images in /build endpoint #9524
[Compat API] Also print successfully tagging images in /build endpoint #9524
Conversation
11ad785
to
4a96d6e
Compare
[NO TESTS NEEDED] Signed-off-by: Riyad Preukschas <[email protected]>
4a96d6e
to
ba319e3
Compare
Should I keep the changes to the swagger docs? |
Yes. |
@rhatdan done. |
LGTM |
@rhatdan Is there anything I can/should do? |
/approve |
lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, riyad The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@riyad Thank you for your work /lgtm |
Thanks 😄 |
Trying to find regressions by exercising the APIv2 trough docker-py's test suite (see #5386) I came across a bug in the
BuildTest::test_build_in_context_dockerfile
test (there're a few tests failing because of the same issue).There're several tests that check for Docker's "summary lines" (e.g. "Successfully ...") in the output stream of the /build endpoint.
Example output from Docker's /build endpoint:
Building the same container with Podman:
Note that the current code already mimics Docker's behavior by additionally sending the "Successfully built ..." line when using the compat API.
This PR "superficially" improves compatibility with Docker by also adding the "Successfully tagged ..." messages.