Fix handling output from tty-enabled containers. #631
Conversation
docker/client.py
Outdated
There was a problem hiding this comment.
This can be replaced with just six.binary_type(), I believe.
Sounds sensible to me.
I'm not sure I fully understand your changes to the tests. Are they now exclusively testing the |
|
@aanand The tests currently don't actually specify a tty setting - the changes I made add a |
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
- Add appropriate test which also asserts that volume names can be passed through to drivers. - Add new param to docs. Signed-off-by: Luke Marsden <luke@clusterhq.com>
as underlying exceptions(such as push already in progress) will be hidden in the stream generator otherwise.
This makes the build method consistent with the events method and adds some convenience.
The boot2docker documentation has since changed the recommended way to use shellinit - see boot2docker/boot2docker#786. The command also no longer prints the export lines to the console, so have updated the example output.
Volume binds now take a "mode" key, whose value can be any string. "ro" is still supported. It is an error to specify both "ro" and "mode". Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
This helps runtime introspection tools like the `help()` builting or IPython's `?` operator correctly find the underlying method instead of the decorator definition.
This makes docker-py consistent with Docker's newish way of establishing the path to .dockercfg: https://github.com/docker/docker/blob/master/pkg/homedir/homedir.go
The current map syntax does not allow the API equivalent of
--add-host foo:1.1.1.1 --add-host foo:2.2.2.2
The above will map one hostname to two IPs. The above is valid in Docker.
This tries to load Docker authentication info from ~/.docker/config.json before falling back to its legacy location and format at ~/.dockercfg. Resolves docker#648
Treat output from TTY-enabled containers as raw streams, rather than as multiplexed streams. The docker API docs specify that tty-enabled containers don't multiplex. Also update tests to pass with these changes, and changed the code used to read raw streams to not read line-by-line, and to not skip empty lines. Addresses issue docker#630 Signed-off-by: Dan O'Reilly <oreilldf@gmail.com>
|
Ugh, it looks like my attempt to squash my commits ended up screwing up the branch completely. I'll open a new request... |
As described in issue #630, getting the stdout/stderr from tty-enabled containers doesn't work properly, because docker-py treats the output as multiplexed, when docker is actually sending a raw stream. This patch attempts to address this by determining if a container to is tty-enabled before trying to read its output, and if it is, read the raw stream, rather than reading at a multiplexed stream.
One piece here I'm not 100% sure about is what to do with the logic that was in
attachto handle old API versions. The logic being used there - read the stream line by line, and skip empty new-lines, doesn't work if the container ever writes blank lines. I also haven't seen any keep-alive new lines being written in my testing, so I wonder if this check isn't really needed?I suppose I could split
_stream_raw_resultinto two methods - one for TTY-enabled containers, and one for versions below 1.6. The one for versions below 1.6 could keep the old, line-by-line behavior.Also please let me know if any additional unit tests are needed.