Skip to content

Conversation

@thaJeztah
Copy link
Member


Previously, the formatter would ignore the quiet option if a custom format
was passed; this situation was handled in runPs(), where custom formats
would only be applied if the quiet option was not set, but only if the
format was set in the CLI's config.

This patch updates NewContainerFormat() to do the same, even if a --format
was passed on the command-line.

This is a change in behavior, so may need some discussion; possible alternatives;

  • produce an error if both --format and --quiet are passed
  • print a warning if both are passed (but use the logic from this patch)

Before this patch:

docker ps --format '{{.Image}}'
ubuntu:22.04
alpine

docker ps --format '{{.Image}}' --quiet
ubuntu:22.04
alpine

mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}" > ~/.docker/config.json

docker ps
ubuntu:22.04
alpine

docker ps --quiet
ubuntu:22.04
alpine

With this patch applied:

docker ps --format '{{.Image}}'
ubuntu:22.04
alpine

docker ps --format '{{.Image}}' --quiet
40111f61d5c5
482efdf39fac

mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}" > ~/.docker/config.json

docker ps
ubuntu:22.04
alpine

docker ps --quiet
40111f61d5c5
482efdf39fac

docker ps: print warning if both --format and --quiet are set

Of both "--quiet" and "--format" are set, --quiet takes precedence. This
patch adds a warning to inform the user that their custom format is not
used:

docker ps --format='{{.Image}}'
ubuntu:22.04
alpine

docker ps --format='{{.Image}}' --quiet
WARNING: Ignoring custom format, because both --format and --quiet are set.
40111f61d5c5
482efdf39fac

The warning is printed on STDERR, so can be redirected:

docker ps --format='{{.Image}}' --quiet 2> /dev/null
40111f61d5c5
482efdf39fac

The warning is only shown if the format is set using the "--format" option.
No warning is shown if a custom format is set through the CLI configuration
file:

mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json

docker ps
ubuntu:22.04
alpine

docker ps --quiet
40111f61d5c5
482efdf39fac

- Description for the changelog

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

Previously, the formatter would ignore the quiet option if a custom format
was passed; this situation was handled in runPs(), where custom formats
would only be applied if the quiet option was not set, but only if the
format was set in the CLI's config.

This patch updates NewContainerFormat() to do the same, even if a `--format`
was passed on the command-line.

This is a change in behavior, so may need some discussion; possible alternatives;

- produce an error if both `--format` and `--quiet` are passed
- print a warning if both are passed (but use the logic from this patch)

Before this patch:

```console
docker ps --format '{{.Image}}'
ubuntu:22.04
alpine

docker ps --format '{{.Image}}' --quiet
ubuntu:22.04
alpine

mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json

docker ps
ubuntu:22.04
alpine

docker ps --quiet
ubuntu:22.04
alpine
```

With this patch applied:

```console
docker ps --format '{{.Image}}'
ubuntu:22.04
alpine

docker ps --format '{{.Image}}' --quiet
40111f61d5c5
482efdf39fac

mkdir -p ~/.docker/
echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json

docker ps
ubuntu:22.04
alpine

docker ps --quiet
40111f61d5c5
482efdf39fac
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f522905)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Of both "--quiet" and "--format" are set, --quiet takes precedence. This
patch adds a warning to inform the user that their custom format is not
used:

    docker ps --format='{{.Image}}'
    ubuntu:22.04
    alpine

    docker ps --format='{{.Image}}' --quiet
    WARNING: Ignoring custom format, because both --format and --quiet are set.
    40111f61d5c5
    482efdf39fac

The warning is printed on STDERR, so can be redirected:

    docker ps --format='{{.Image}}' --quiet 2> /dev/null
    40111f61d5c5
    482efdf39fac

The warning is only shown if the format is set using the "--format" option.
No warning is shown if a custom format is set through the CLI configuration
file:

    mkdir -p ~/.docker/
    echo '{"psFormat": "{{.Image}}"}' > ~/.docker/config.json

    docker ps
    ubuntu:22.04
    alpine

    docker ps --quiet
    40111f61d5c5
    482efdf39fac

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 37e02ff)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.84%. Comparing base (0b3f540) to head (de7e265).
Report is 177 commits behind head on 23.0.

Additional details and impacted files
@@           Coverage Diff           @@
##             23.0    #4201   +/-   ##
=======================================
  Coverage   58.83%   58.84%           
=======================================
  Files         572      572           
  Lines       49610    49620   +10     
=======================================
+ Hits        29188    29198   +10     
  Misses      18648    18648           
  Partials     1774     1774           

@thaJeztah
Copy link
Member Author

Discussing with @neersighted whether or not to backport this to v23.0;

👍 fixes docker ps --quiet with a custom format in ~/.docker/config.json
👎 it's a change in behaviour (notably for docker ps --format {''some format"}} --quiet)

/cc @tianon @vvoland @cpuguy83 @AkihiroSuda WDYT?

@tianon
Copy link
Contributor

tianon commented Apr 13, 2023

No strong feelings - I don't think it needs a backport, but I think the new behavior is much more correct (and matches the user expectation better), so it's totally reasonable to backport if 23.x is going to continue to live for a while.

@thaJeztah
Copy link
Member Author

FWIW; I discussed with @neersighted and he was okay with this change, but wanted to have a quick peek from others

@AkihiroSuda
Copy link
Collaborator

For v23, it might be safer to just retain the old behavior but print a warning message

@thaJeztah
Copy link
Member Author

Yeah, I guess the "complicated" one is the case where both as passed through --flags. The case where a default is configured in the ~/.docker/config.json (and --quiet not working) is easier.

I could consider adding a commit that only changes the behaviour for ~/.docker/config.json for 23.0, and we can decide on the other ones later (or keep it)

@thaJeztah thaJeztah marked this pull request as draft April 14, 2023 10:24
@thaJeztah thaJeztah modified the milestones: 23.0.4, 23.0.5 Apr 14, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.5, 23.0.6 Apr 26, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.6, 23.0.7 May 5, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.7, 23.0.8 Jul 17, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.8, 23.0.7 Sep 14, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.7, 23.0.8 Sep 28, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.8, 23.0.9 Dec 6, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.9, 23.0.10 Jan 31, 2024
@thaJeztah thaJeztah closed this Oct 19, 2024
@thaJeztah thaJeztah removed this from the 23.0.10 milestone Oct 19, 2024
@thaJeztah thaJeztah deleted the 23.0_backport_ps_always_accept_quiet branch October 19, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants