Skip to content

Conversation

aanand
Copy link
Contributor

@aanand aanand commented Apr 23, 2014

Docker introduced newlines in stream output in version 0.9 (moby/moby#4276), but not to all endpoints - POST /images/create, for example, does not include them. This means that #184, though it fixed #176, introduced a regression in client.pull('ubuntu', stream=True), as reported in #199.

This reverts to the old, less pleasant implementation of _stream_helper(), with a manual check for newlines to fix the original problem, without the accompanying regression. It should work against Docker 0.8, 0.9 and 0.10, both when building and when pulling.

Fixes #199.

@shin-
Copy link
Contributor

shin- commented Apr 23, 2014

Feels like we've gone back and forth a few times on this method already. Have you had a chance to test this under both Python 2 and 3?

@aanand
Copy link
Contributor Author

aanand commented Apr 23, 2014

I haven't tested it under Python 3, no. Is there already a simple way to do so inside a Docker container? (I'll install Python 3 in a VM if I absolutely have to, but I'd rather avoid it).

@aanand
Copy link
Contributor Author

aanand commented Apr 28, 2014

OK, I've added checks to the integration test and can confirm that it runs under Python 3.3 (a flurry of failures in other tests notwithstanding; think that file needs a bit of love in general.)

Docker introduced newlines in stream output in version 0.9
(moby/moby#4276), but not to all
endpoints - POST /images/create, for example, does not include them.

This reverts to the old, less pleasant implementation of
_stream_helper(), with a manual check for newlines to fix the problem
described in docker#176 and fixed in docker#184, without the accompanying
regression. It should work against Docker 0.8, 0.9 and 0.10, both when
building and when pulling.
@shin-
Copy link
Contributor

shin- commented Apr 28, 2014

LGTM

@mpetazzoni
Copy link
Contributor

We've been back and forth on this several times indeed. If this has been successfully tested (on all methods!) with both Python 2.x and 3.x, talking to Docker daemons 0.7, 0.8, 0.9 and 0.10, then I'm ok with the change.

@tifayuki
Copy link

This works well on 0.11. we need this fix.

@aanand
Copy link
Contributor Author

aanand commented May 14, 2014

I've tested it by hand by running the integration test against 0.8.0, 0.9.0 and 0.10.0 and Python 2.7/3.3, but is there a more robust/automated way to do it? (Related: other parts of the integration test break for me - is it maintained?)

@mpetazzoni
Copy link
Contributor

LGTM then. Thanks a lot!

mpetazzoni added a commit that referenced this pull request May 21, 2014
Universally-compatible reading of chunked streams
@mpetazzoni mpetazzoni merged commit 55b93d6 into docker:master May 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

client.pull(stream=True) doesn't split stream lines correctly? streaming from build() is broken in 0.3.0

5 participants