fix docker info, docker version --format=json not outputting json format #4168
Conversation
thaJeztah
added
status/2-code-review
process/cherry-pick
kind/bugfix
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4168 +/- ##
==========================================
+ Coverage 59.03% 59.04% +0.01%
==========================================
Files 288 288
Lines 24776 24775 -1
==========================================
+ Hits 14627 14629 +2
+ Misses 9265 9262 -3
Partials 884 884 |
Signed-off-by: Sebastiaan van Stijn <[email protected]>
The --format=json option was added for all inspect commands, but was not implemented
for "docker info". This patch implements the missing option.
Before this patch:
docker info --format=json
json
With this patch applied:
docker info --format=json
{"ID":"80c2f18a-2c88-4e4a-ba69-dca0eea59835","Containers":7,"ContainersRunning":"..."}
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
docker_info_json
branch
from
ed1c775 to
46234b8
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
The --format=json option was added for all inspect commands, but was not
implemented for "docker version". This patch implements the missing option.
Before this patch:
docker version --format=json
json
With this patch:
docker version --format=json
{"Client":{"Platform":{"Name":""},"Version":"24.0.0-dev","ApiVersion":"..."}}
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
changed the title
There was a problem hiding this comment.
Sneaky little commit that's unrelated :)
There was a problem hiding this comment.
It's a .golden file, and used for the extra test-case that was added;
assert.Check(t, golden.String(cli.OutBuffer().String(), "docker-client-version.json.golden"))There was a problem hiding this comment.
😂 so I thought you meant that JSON file, but it was referring to the "make it a const" commit. I had that in another PR, but doing it later would mean we had more code-churn, as the lines using it are touched by this PR, so I moved it first to prevent that.
| } | ||
|
|
||
| func formatInfo(dockerCli command.Cli, info info, format string) error { | ||
| if format == formatter.JSONFormatKey { |
There was a problem hiding this comment.
This is unfortunate, changing the parameter on fly is not great
There was a problem hiding this comment.
Yeah, the system info printing is quite involved currently. I can use a new variable for it if you prefer but thought to keep it simple (it's not passed by reference in either case, so only used here).
There was a problem hiding this comment.
Don't we want the system info formatting to go to the formatting package?
There was a problem hiding this comment.
Not sure, we'd have to look. I think there's been some back-and-forth in the past; a single "formatting" package would also have to import all the types used by all objects, so I think it originally was a single package, then got split to each object type. to allow for specific logic for each where needed.
Definitely something to look at what the current state is, and if things are all in the right place.
3 participants
fix docker info --format=json not outputting json format
The --format=json option was added for all inspect commands, but was not implemented for "docker info". This patch implements the missing option.
Before this patch:
With this patch applied:
fix docker version --format=json not outputting json format
The
--format=jsonoption was added for all inspect commands, but was notimplemented for "docker version". This patch implements the missing option.
Before this patch:
With this patch:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)