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

adding STDOUT and STDERR documentation #313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,20 @@ context, the Docker object is removed automatically, even if an exception occurs
* All Docker objects and the Docker client are safe to use with multithreading and multiprocessing.
* Display the commands called and the environment variables used by setting the environment variable `PYTHON_ON_WHALES_DEBUG=1`.

### STDOUT and STDERR handling

_While this is by no means an exhaustive explanation it conveys the overall philosophy of python-on-whales_

Most python-on-whales methods capture `STDOUT` and `STDERR`.

`STDERR` is only used to raise appropriate exceptions when the underlying docker process fails.

Many of these methods, like `docker.volume.prune`, return `STDOUT` in its raw form. You can ignore the `STDOUT` or perhaps log it.

Choose a reason for hiding this comment

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

That's not really true. I don't know if it was the purpose of your last PR, but stdout is not returned. If it were to be returned (which is a valid thing to do), we would parse stdout and output a list of volume IDs (strings).

In you last PR, there was run(full_cmd), to return stdout, we would need to do return run(full_cmd), but I believe the best thing to do would be

output = run(full_cmd)
if output == "":
    return []
else:
    return output.splitlines()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry. I missed something. utils.run returns stdout but docker.volume.prune does not return utils.run.

Agreed it could be nice to do the above, which is what I thought happened when I wrote this addition to the README. It seems a little involved but entirely doable.

Looks like this PR will get a lot bigger, and probably not be done for a bit. :)

(Also, I wrote my previous PR because volume.prune's output was appearing in my program's stdout while no other prune method's was.)

Choose a reason for hiding this comment

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

No worries. I'm not sure many people use the output of docker.volume.prune. So don't worry about it. You can just remove the section concerning docker.volume.prune in this PR and then we can merge it :)


Certain of these methods, e.g. `docker.container.list`, use the captured `STDOUT` to return objects more specific to docker, e.g. `List[Container]`.

Other python-on-whales methods by default do not capture `STDOUT` and `STDERR`. Instead these methods output them directly to the terminal to better handle more complicated output. One such method is `docker.buildx.build`.

## Why another project? Why not build on Docker-py?

In a sense this project is built on top of [Docker-py](https://docker-py.readthedocs.io/en/stable/)
Expand Down