return stdout & stdin when using docker compose

[Sid-Sun]

patch introduces support to return captured stdout & stdin when doing pull, down, up, build (targets #395)

had to invert the quiet in some places because:
run expects a True if it is to be captured but
wrapper sends quiet as false when it is to be captured

enable capturing on build and pull, if needed, client may set --quiet flag via the quiet option (note: not the same as quiet on up and down) and simply ignore returned value(s)

[gabrieldemarmiesse]

Thank you for the pull request. Since this is touching the public api in a non-trivial manner, I'll give myself some time to think about it. Since we want to garantee backward compatibility, we don't want to put ourselves back against a wall

[Demoli]

Further to this it seems docker compose actually writes to STDERR instead of STDOUT in many cases.
docker/compose#7346

When I apply @Sid-Sun's patch locally and run a DockerClient::docker.compose.up the stdout is captured but it's typically empty so nothing is returned by the run method.

If I pass return_stderr to the run method in
components/compose/cli_wrapper.py:690
like so
return run(full_cmd, capture_stdout=(not quiet), capture_stderr=(not quiet), return_stderr=True)

It returns a list with stdout and stderr which I can then read.

It would be great to also be given the option to capture_stderr with a new parameter on the various compose methods.

[Sid-Sun]

Thanks for pointing this out.

I wasn't aware of the return_stderr parameter, somewhat strange that capture doesn't necessarily imply return.

Nevertheless if the change in API is acceptable, we can set return parameters to true and get the outputs.

[Demoli]

The capture arguments are correct in that they tell the run method to capture the subprocess output, but then the output is passed to post_process_stream.

If return_stderr isn't passed then it only processes stdout and not stderr.
python_on_whales/utils.py:184

I agree that the return_stderr param seems redundant, and perhaps capturing should always imply return.

[gabrieldemarmiesse]

I'm trying to understand the use case a bit better to make sure we get the API right.

I totally understand the need for stderr and stdout when those are coming from a container (e.g. docker.compose.up() or docker.compose.run()). But what's the need for other functions like build or pull? Isn't a quiet-not quiet option enough? As long as the user can see the progress in the terminal it should be enough no? Or maybe i'm missing a use case entirely

[Demoli]

My specific use case is a terminal user interface which takes over the entire terminal (like htop or a curses app).

Anything that writes directly to the terminal breaks the rendering of my app.

I'm only using up and down at the moment, for something like build which takes a while I'd need direct access to the subprocess output stream rather than waiting for it to complete.

[HemaZ]

+1 For this PR. I'm building a GUI app which interacts with docker compose a lot and i would like to be able to capture the stderr/stdout to analyse in case of errors.

[gabrieldemarmiesse]

Sorry for the silence on my part. I'd totally forgotten about that issue, which is non-trivial. I'll take a look tomorrow and continue this PR, or create another one to have a better handling of stderr and stdout.

[gabrieldemarmiesse]

Ok so I took a look at it, and here is how it should go.
Every compose fonction that logs something interesting (up, exec, run, push, pull, start, build) should have a new argument (or rename the existing one): stream_logs. When this is enabled, an iterator is returned. This iterator is a tuple[str, bytes]. You can see how this iterator looks like here: https://gabrieldemarmiesse.github.io/python-on-whales/user_guide/docker_run/#stream-the-output

If the user set in any function stream_logs=True and quiet=True at the same time, Python-on-whales should throw an error saying that it's not possible to have both arguments being True at once.

When stream_logs=False, then either None is returned, or the already existing object, like in the function docker.compose.run() which already does that and returns a Container when not streaming.

If a function already has a stream argument, then this argument must be changed to stream_logs.

All of that is possible with the function stream_stdout_and_stderr. An example can be found with docker.compose.run().

To help with the conditional type return, we should use the typing.overload decorator. It will allow mypy, pyright, vscode and pycharm tell the user when the iterator is returned or when something else is returned depending on the input arguments. So the conditional type on return should not be a big deal.

Does that work for all people involed who wanted this feature?

[HemaZ]

Ok so I took a look at it, and here is how it should go. Every compose fonction that logs something interesting (up, exec, run, push, pull, start, build) should have a new argument (or rename the existing one): stream_logs. When this is enabled, an iterator is returned. This iterator is a tuple[str, bytes]. You can see how this iterator looks like here: https://gabrieldemarmiesse.github.io/python-on-whales/user_guide/docker_run/#stream-the-output

If the user set in any function stream_logs=True and quiet=True at the same time, Python-on-whales should throw an error saying that it's not possible to have both arguments being True at once.

When stream_logs=False, then either None is returned, or the already existing object, like in the function docker.compose.run() which already does that and returns a Container when not streaming.

If a function already has a stream argument, then this argument must be changed to stream_logs.

All of that is possible with the function stream_stdout_and_stderr. An example can be found with docker.compose.run().

To help with the conditional type return, we should use the typing.overload decorator. It will allow mypy, pyright, vscode and pycharm tell the user when the iterator is returned or when something else is returned depending on the input arguments. So the conditional type on return should not be a big deal.

Does that work for all people involed who wanted this feature?

Seems amazing to me. Thanks

[gabrieldemarmiesse]

Hi people, it took some time, but I started working on this. See #494 . I'll work on doing the same with the other functions.

[gabrieldemarmiesse]

#496 for docker.compose.down()

[gabrieldemarmiesse]

I'll close this pull request, you can follow the progress in this issue: #499