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

Conversation

dvizzini
Copy link
Contributor

Converts #311 (comment) to README form.


`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 :)

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.

2 participants