Skip to content

Added support for docker push --quiet#1221

Closed
justyntemme wants to merge 1 commit intodocker:masterfrom
justyntemme:master
Closed

Added support for docker push --quiet#1221
justyntemme wants to merge 1 commit intodocker:masterfrom
justyntemme:master

Conversation

@justyntemme
Copy link
Copy Markdown
Contributor

Signed-off-by: Justyn Temme justyntemme@gmail.com

- What I did

Added --quiet flag for docker push
Fix for #958

- How I did it

Passing a flag to either write out the content body of the response, or simply output nothing

- How to verify it
docker push --quiet tagged/image
- Description for the changelog

Added a --quiet flag for docker push

- A picture of a cute animal (not mandatory but encouraged)

cute bunny

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 18, 2018

Codecov Report

Merging #1221 into master will increase coverage by <.01%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #1221      +/-   ##
==========================================
+ Coverage   54.25%   54.25%   +<.01%     
==========================================
  Files         268      269       +1     
  Lines       17799    17805       +6     
==========================================
+ Hits         9656     9660       +4     
- Misses       7536     7538       +2     
  Partials      607      607

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Thanks! I think this needs some more changes; see my inline comment

Comment thread cli/command/image/push.go
defer responseBody.Close()
return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil)
if !opts.quiet {
return jsonmessage.DisplayJSONMessagesToStream(responseBody, dockerCli.Out(), nil)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of returning nothing, we should make the output useful, so that it can be used when chaining commands (e.g. IMAGE=$(docker image push -q foobar/foo:1.1.1); docker service create --name foo $IMAGE).

See my comment here; moby/moby#13588 (comment), and this PR; #882, which is implementing this for docker image pull

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

--quiet now prints the fqn for the image in the repository

Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

I'm good with the code, but I think we need a test on this feature (see cli/command/image/push_test.go) to be complete 😄

@justyntemme
Copy link
Copy Markdown
Contributor Author

justyntemme commented Jul 25, 2018

I will write one up today, thanks! Unit test written. It simply checks if the command with the --quiet argument completes with no errors. I mirrored the existing push test.

Signed-off-by: Justyn Temme <justyntemme@gmail.com>
@thaJeztah
Copy link
Copy Markdown
Member

Thanks for updating! Just gave this a spin;

$ docker pull hello-world
Using default tag: latest
latest: Pulling from library/hello-world
9db2ca6ccae0: Pull complete 
Digest: sha256:4b8ff392a12ed9ea17784bd3c9a8b1fa3299cac44aca35a85c90c5e3c7afacdc
Status: Downloaded newer image for hello-world:latest

$ docker tag hello-world localhost:5000/foobar
$ docker push --quiet localhost:5000/foobar
localhost:5000/foobar

I want to briefly discuss with other people if we want the digest to be somewhere in the output, because this is the only moment where you'll be able to get the registry-digest when pushing a new image (I can see this being important for people to be able to later on verify/pull the exact same image)

@thaJeztah thaJeztah changed the title Added support for --quiet Added support for docker push --quiet Jul 30, 2018
args: []string{"--quiet", "image:tag"},
},
}
for _, tc := range testCases {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for this test! But I think you can safely remove this for pattern as you have only one test case 😸

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Will change pr within the hour.

@justyntemme
Copy link
Copy Markdown
Contributor Author

No problem at all, will edit test as requested and wait to hear back on implementation details.

@sidcarter
Copy link
Copy Markdown

Is there a plan to push this at some point? :)

facebook-github-bot pushed a commit to pytorch/pytorch that referenced this pull request Jul 20, 2019
Summary:
I couldn't find any verbosity options in the [`docker pull` command docs](https://docs.docker.com/engine/reference/commandline/pull/), but
`docker pull` [got a `--quiet` option](docker/cli#882) in a recent version (not sure if we're using that version), and `--quiet` for `docker push` [is forthcoming](docker/cli#1221).
Pull Request resolved: #23111

Differential Revision: D16402993

Pulled By: kostmo

fbshipit-source-id: 52f77b11b839d28f8cf1ecb58518ca69632d7fbe
@cadavre
Copy link
Copy Markdown

cadavre commented Nov 6, 2019

Ping. How about this PR? Would love to see both pull and push with -q option for CI/CD purposes where we now have 250 lines of downloading/extracting...

@thaJeztah
Copy link
Copy Markdown
Member

carrying in #2197

@jwhipp
Copy link
Copy Markdown

jwhipp commented Feb 5, 2020

What version was this included in? Not seeing it as a valid option in Docker version 19.03.5, build 633a0ea.

@cpuguy83
Copy link
Copy Markdown
Collaborator

cpuguy83 commented Feb 5, 2020

It is not part of any release yet.

@kperry42
Copy link
Copy Markdown

I'd just like to "second" getting this into a released version. Our CI/CD log is chock full of pointless "Preparing" messages, etc.

@thaJeztah
Copy link
Copy Markdown
Member

This will be in the upcoming 20.x release.

Our CI/CD log is chock full of pointless "Preparing" messages, etc.

For CI/CD, if you don't want the output, just piping to /dev/null should work

docker pull ubuntu  > /dev/null

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.