Skip to content
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

align docker compose ps with docker ps #10918

Merged
merged 2 commits into from Aug 25, 2023
Merged

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Aug 22, 2023

What I did
Use the docker/cli formatter to implement docker compose ps so that --format offers the same templating options
Note: this is a breaking change as --format "json" used to produce a json array, while docker/cli produces Newline-separated json stream.

Related issue
closes #10874
https://docker.atlassian.net/browse/ENV-282

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@ndeloof ndeloof force-pushed the align_ps_cli branch 2 times, most recently from 046323d to c937ba3 Compare August 22, 2023 14:13
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Patch coverage: 69.40% and project coverage change: +0.06% 🎉

Comparison is base (bc9d696) 58.14% compared to head (769f581) 58.20%.
Report is 2 commits behind head on main.

❗ Current head 769f581 differs from pull request most recent head c0c6768. Consider uploading reports for the commit c0c6768 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10918      +/-   ##
==========================================
+ Coverage   58.14%   58.20%   +0.06%     
==========================================
  Files         121      122       +1     
  Lines       10693    10776      +83     
==========================================
+ Hits         6217     6272      +55     
- Misses       3862     3887      +25     
- Partials      614      617       +3     
Files Changed Coverage Δ
pkg/api/api.go 42.30% <ø> (ø)
cmd/compose/ps.go 67.01% <64.70%> (-6.04%) ⬇️
cmd/formatter/container.go 67.32% <67.32%> (ø)
cmd/compose/compose.go 70.90% <87.50%> (ø)

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ndeloof ndeloof requested review from a team, nicksieger, StefanScherer, ulyssessouza, glours, milas and laurazard and removed request for a team August 22, 2023 14:59
Copy link
Member

@milas milas left a comment

Choose a reason for hiding this comment

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

LGTM! I only ran into one issue, which is there's no truncation on COMMAND.

Current:

❯ docker compose ps -a
NAME                IMAGE               COMMAND                  SERVICE             CREATED             STATUS              PORTS
pstest-fib-1        debian              "bash -c 'function f…"   fib                 3 seconds ago       Up 3 seconds        0.0.0.0:57185->34123/tcp
pstest-sleep-1      debian              "sleep infinity"         sleep               2 minutes ago       Up 2 minutes

This PR:

❯ ~/dev/compose/bin/build/docker-compose ps -a
NAME           IMAGE     COMMAND   SERVICE   CREATED   STATUS    PORTS
pstest-fib-1   debian    bash -c 'function fib(){
    if [ $1 -le 0 ]; then
        echo 0
    elif [ $1 -eq 1 ]; then
        echo 1
    else
        echo $[`fib $[$1-2]` + `fib $[$1 - 1]` ]
    fi

}

fib $THIS_IS_THE_FIBONACCI_NUMBER
'                fib       13 seconds ago   Up 12 seconds   0.0.0.0:57185->34123/tcp
pstest-sleep-1   debian    sleep infinity   sleep           3 minutes ago   Up 3 minutes

(yes I copy pasted a shell script to calculate the fibonacci sequence as an inline command in Compose YAML)

docker ps:

❯ docker ps
CONTAINER ID   IMAGE         COMMAND                  CREATED          STATUS          PORTS                      NAMES
0657d4b4fcd6   debian        "bash -c 'function f…"   37 seconds ago   Up 36 seconds   0.0.0.0:57185->34123/tcp   pstest-fib-1
1b9380a354f1   debian        "sleep infinity"         3 minutes ago    Up 3 minutes                               pstest-sleep-1
name: pstest

services:
  fib:
    image: debian
    init: true
    ports:
      - 34123
    command:
      - bash
      - -c
      - |
        function fib(){
            if [ $$1 -le 0 ]; then
                echo 0
            elif [ $$1 -eq 1 ]; then
                echo 1
            else
                echo $$[`fib $$[$$1-2]` + `fib $$[$$1 - 1]` ]
            fi

        }

        fib $$THIS_IS_THE_FIBONACCI_NUMBER
    environment:
      - THIS_IS_THE_FIBONACCI_NUMBER=${THIS_IS_THE_FIBONACCI_NUMBER:-100}
  sleep:
    image: debian
    init: true
    command: [sleep, infinity]

@ndeloof
Copy link
Contributor Author

ndeloof commented Aug 24, 2023

@milas indeed, missed a copy/paste from docker/cli :P
Also added support for other attributes exposed by docker ps which we didn't included in the backend.Ps API (yet another legacy from ECS integration...)

@ndeloof ndeloof force-pushed the align_ps_cli branch 4 times, most recently from fa7d606 to c26ea3f Compare August 24, 2023 13:47
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@tuler
Copy link

tuler commented Sep 21, 2023

Does Docker compose follow semver?

This is clearly a breaking change, and it actually broke stuff that expects a certain behavior.

@ndeloof
Copy link
Contributor Author

ndeloof commented Sep 21, 2023

Does Docker compose follow semver?

nope, at least not in the sense "no single backward incompatible change will happen without a major version bump"

@artshade
Copy link

artshade commented Mar 19, 2024

Dear Developers,

Sincere... heartfelt... gratitude for the ineffably marvelous project and the work, contribution you do (really)...


I am sorry, but I just... I just... have to express it... My dear... It's been days I've trying to figure out what was causing docker compose ps JSON parsing to fail...

Apparently, THIS did.

ALL the tests on production, staging, and test environments were checking whether Docker Compose returns a valid 64 LENGTH string, and the command itself was:

docker compose ps --format 'json' -- php;

Sure thing it returned 64 characters and there were tests ^[0-9a-f]{64}$ applied for the output, and sure it ALL FAILED all of a sudden! And, it's been 7 MONTHS apparently since the change, yet we have upgraded just recently...

Now, apparently, it is:

docker compose ps --no-trunc --format=json -- php;

I mean... JSON is non-human format frankly... even if it is fairly readable! It was specifically created to transfer/store machine data, wasn't it?! Why would you want to send short format of such important data like CONTAINER ID in JSON? HOW WILL those 52 BYTES SAVE THE WORLD when someone EXPLICITLY asks for JSON... in non-interactive shell call?!


JSON (JavaScript Object Notation) is a lightweight data-interchange format. It is easy for humans to read and write. It is easy for machines to parse and generate. It is based on a subset of the JavaScript Programming Language...

Source

It is a commonly used data format with diverse uses in electronic data interchange, including that of web applications with servers.

Source

Uhh... JSON Statham...
Uhh... I am sorry...

🕊️


Best and kind regards ✨

@ndeloof
Copy link
Contributor Author

ndeloof commented Mar 20, 2024

JSON Statham

😅

Now, apparently, it is:
docker compose ps --no-trunc --format=json

indeed.
This just aligns with docker ps --format json, so this is not a new thing json output truncates IDs, whenever this is not designed for human usage and long ID would be a reasonable default.

@thaJeztah
Copy link
Member

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.

docker compose ps --format should be configurable by ~/.docker/config.json.
5 participants