-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add GO templating capabilities to images and list commands #12949
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
base: main
Are you sure you want to change the base?
Conversation
Hello @leoperegrino |
LGTM regarding the code approach
Assuming the final goal is to align with docker/cli, format should offer the same data as https://github.com/docker/cli/blob/master/cli/command/formatter/image.go#L198-L208 and reuse existing public types I'm not convince alignment with docker/cli is the best option here, and go templating format support is even desirable (with json output, you can pipe to a dedicated formatting engine, typically |
e594b0a
to
63055a2
Compare
Sorry about the signoff, I thought it was done automatically but already push forced amended commits. The use case for this feature was expanded a bit more in the issue #12948. It consists in avoiding the use of a external tool such as I don't think it needs to be a 1-1 mapping to docker/cli, I just tried to not deviate from the current codebase. But if a specific data structure format is required, I could change it when agreed upon. Besides the slice/array difference, I think the only change in the attributes it's the ID. Now it's going to be truncated unless the |
feel free to change "api" so that it returns |
The When formatting as json should they appear? Because right now, they don't. docker compose ls --format json
# [{"Name":"test_1","Status":"running(2)","ConfigFiles":"..."},{"Name":"test_2","Status":"running(1)","ConfigFiles":"..."}]
|
|
This is inspired by docker/cli and 1054792 Signed-off-by: Leonardo Peregrino <[email protected]>
This is inspired by docker/cli and 1054792 Signed-off-by: Leonardo Peregrino <[email protected]>
63055a2
to
5a0ca60
Compare
I've made the Lines 128 to 133 in 5a0ca60
Lines 129 to 133 in 5a0ca60
Now, it's possible to access the Lines 181 to 186 in 5a0ca60
Lines 154 to 159 in 5a0ca60
docker compose images --format json
# {"ContainerName":"test_1-compose_test-1","Created":"292 years ago","ID":"sha256:350b164e7ae1dcddeffadd65c76226c9b6dc5553f5179153fb0e36b78f2a5e06","LastTagTime":"0001-01-01 00:00:00 +0000 UTC","Platform":"linux/amd64","Repository":"gcr.io/google-containers/pause","Size":"239840","Tag":"latest"}
# {"ContainerName":"test_1-compose_test_2-1","Created":"292 years ago","ID":"sha256:350b164e7ae1dcddeffadd65c76226c9b6dc5553f5179153fb0e36b78f2a5e06","LastTagTime":"0001-01-01 00:00:00 +0000 UTC","Platform":"linux/amd64","Repository":"gcr.io/google-containers/pause","Size":"239840","Tag":"latest"}
docker compose images --format table
# CONTAINER REPOSITORY TAG PLATFORM IMAGE ID SIZE CREATED
# test_1-compose_test_2-1 gcr.io/google-containers/pause latest linux/amd64 350b164e7ae1 240kB 292 years ago
# test_1-compose_test-1 gcr.io/google-containers/pause latest linux/amd64 350b164e7ae1 240kB 292 years ago
docker-compose ls --format json
# {"ConfigFiles":"...","Name":"test_1","Status":"running(2)"}
# {"ConfigFiles":"...","Name":"test_2","Status":"running(1)"}
docker compose ls --format table
# NAME STATUS CONFIG FILES
# test_1 running(2) ...
# test_2 running(1) ... I believe this should be enough but tell me what do you think. |
for JSON support, sounds way simpler to just bypass all the formater logic and rely on json.Marshall, or did I missed something ? |
Could definitely be, I tried to mimic already inplace behavior and that's why I used the Context approach. |
yes, adopt docker/cli approach with the minimal amount of code (feel free to make a few changes so you can directly use the same structs/functions) + include a dedicated if block to manage JSON output for backward compatibility |
Ok but to maintain the templating capability we would also need all the Context code, right? So if that code is already going to be there, why not reuse for the JSON marshal? This is how it's done by the ps command where it simply calls the ContainerWrite regardless if it was passed a JSON format or not: Lines 154 to 160 in 4f491ff
To me this makes more sense than to have another logical path in the run command just for JSON |
What I did
Implemented GO templates for images and list commands. I tried to align the changes with already existing commits, such as 1054792, even though a new formatter file in
cmd/formatter/<command>.go
was not created.This PR changes the output of the json format. This is the default behavior for the commands that have template support.
For the images command, a new
--no-trunc
flag was added to control the truncate behavior of the image ID.Related issue
This closes #12948.
(not mandatory) A picture of a cute animal, if possible in relation to what you did