Fix to enable streaming container logs reliably

[dims]

This patch is based on @kevinastone 's code in:
#300 (comment)

Started a ubuntu container that just runs "ping 8.8.8.8" and tried
the sample code in https://gist.github.com/dims/c3327f633c526847c8e5
to recreate the problem mentioned in:
#300

To debug the problem i printed the byte array read in recvall
when reading STREAM_HEADER_SIZE_BYTES and realized that the data
being read was far ahead of the actual start of the header documented
in the vnd.docker.raw-stream of the docker remote api. This is
possibly because the requests/urllib3 is reading ahead a bit more
and we shouldn't be trying to hack the internals of those projects.
So just using the documented file-like response.raw is good enough
for us to get the functionality we need which is being able to
read for exactly where the stream header starts. With this change
i can reliably stream the logs just like "docker logs --follow".

[objectified]

+1 for this patch. I've applied this patch in a project I'm working on, and it works great. Do keep in mind that the newline for every log line is not stripped by docker-py, so simply printing/logging the line will add an extra newline (unless you explicitly deal with it).

[objectified]

While working with this patch some more, I've discovered that this often leads to a read timeout when no data is being returned for a while (for example a long running Docker process).

File "/Users/lbrouwer/dev/vdist/lib/python2.7/site-packages/docker/client.py", line 323, in _multiplexed_response_stream_helper
header = response.raw.read(STREAM_HEADER_SIZE_BYTES)
File "/Users/lbrouwer/dev/vdist/lib/python2.7/site-packages/requests/packages/urllib3/response.py", line 210, in read
raise ReadTimeoutError(self._pool, None, 'Read timed out.')
requests.packages.urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host='boot2docker', port=2376): Read timed out.

[kevinastone]

Yeah, you need to disable the timeout on the underlying socket. I have this in my version of the code:

        sock = stream._fp.fp._sock
        sock.settimeout(None)
        self.stream = stream

(I believe stream is response.raw)

[dims]

@kevinastone added the settimeout in my patch. thanks!

@objectified can you please try the updated patch?

[objectified]

Thanks, the updated patch seems to fix this problem. I ran a number of tests with running/stalling/running/stalling/etc. behavior inside a few different docker containers, and I could not reproduce the original problem.

[dims]

@objectified thanks for the quick response!

[dims]

@shin- Any feedback on this PR? Please let me know.

[shin-]

Sorry for the delay. Looks good, any reason you didn't entirely replace _multiplexed_socket_stream_helper()? Also, have you tested it with Python 2 and 3?

[objectified]

Good point about Python 3 - I've just tested this functionality using Python 3, and one thing I noticed while doing so is that the yielded data is a byte object, not a string. I'm not sure if that's intentional, but since this functionality is used to get logging from a container, I'd assume a string type would make the most sense. It can be easily fixed by replacing:

    yield data

with:

    yield data.decode('utf-8')

But again, I don't know whether this is intentional.

[shin-]

Hey @objectified , thank you for testing! this is not a change brought up by this PR. We've always yielded bytestrings in Py3.

[objectified]

@shin- Fair enough.
@dims Any chance you could elaborate on the point raised by @shin about replacing the original method? It'd be great if this pull request could be merged.

[dims]

@objectified Am on it. thanks for the poke

[dims]

@shin-, @objectified - tested with python2.7 and python3.4 using this snippet - http://paste.openstack.org/show/158892/

[objectified]

Thanks @dims ! I've tested it with my project on 2.7 and 3.4 as well, and it appears to work nicely.

[objectified]

@shin- Seems your input is processed now. If there's anything else related to this that needs testing or anything else that needs to be fixed, could you let us know? If this PR could be merged, that would be great.

[dims]

@shin- ping :)

[shin-]

Sorry guys, I just want to run some tests on my end with this before merging, and I haven't had the time so far. This weekend at the latest. Thanks a lot for your help, and your patience.

[dims]

Thanks @shin- looking forward to feedback

[objectified]

@shin- did you have a chance to run your tests yet?

Sorry for hunting this one, I need this pull request merged so that my own project will work (which I can then open source).

[shin-]

Alright, sorry about the delay! This all looks good :) Thanks a ton for your contribution!

[shin-]

0.7.2 released with that fix :)

[dims]

Thanks @shin-