-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Support python 3 #440
Support python 3 #440
Conversation
@@ -227,7 +227,7 @@ def _stream_helper(self, response): | |||
# Because Docker introduced newlines at the end of chunks in v0.9, | |||
# and only on some API endpoints, we have to cater for both cases. | |||
size_line = socket.readline() | |||
if size_line == '\r\n': | |||
if size_line == '\r\n' or size_line == '\n': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix is already upstream, so it should be "ok" to make this even in a vendored package:
https://github.com/docker/docker-py/blob/master/docker/client.py#L274
Sorry for the delay checking this out. Life has been a bit hectic lately. I'm just catching up on emails etc now. I probably don't have time to look at the dockerpty thing until tomorrow evening, but it's on my radar, thanks. |
Looks great, needs a rebase though. I wonder if we can use tox on travis: http://www.dominicrodger.com/tox-and-travis.html |
@dnephin Are you going to rebase the PR in the near future? Otherwise someone else might pick it up and create a new PR against the current tip. |
d68ad46
to
a23629e
Compare
Rebase complete |
Oops – needs another rebase. Sorry! |
@dnephin Can you please do another rebase? |
rebased, tests still running, so not sure if I need to make any other changes |
There are a bunch of failures on 3.4 now. Some things that need to be imported from six.moves, etc |
Hey fig collaborators, do you need some help on this? I don't use python3 but I would like to help since I use fig daily. |
@mauricioabreu Help with Python 3 would be great, yes. |
hey @dnephin I created a pull request against your python3 branch. |
@mauricioabreu thanks! I merged them in, but the tests didn't run because of a missing signoff. We'll have to rebase this branch with signed-off commits. |
I am sorry for this. I think a lot of people are not used to commit using sign-off feature. I am glad to help anyways. :) |
+1 on this, need it, anything that can be done to help this get merged up? |
I've cherry-picked my original commit on the latest master. |
b35ff26
to
de8cbf2
Compare
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
In particular it includes: - some extension of CONTRIBUTING.md - one fix for Python 2.6 in tests/integration/cli_test.py - one fix for Python 3.3 in tests/integration/service_test.py - removal of unused imports Make stream_output Python 3-compatible Signed-off-by: Frank Sachsenheim <funkyfuture@riseup.net>
… for py2 and py3. Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
…r-py response. Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
The tests seem to be passing, there was a couple flakes against 1.8.1 on the last run, so I'm re-running them now. |
from . import colors | ||
from .multiplexer import Multiplexer | ||
from .utils import split_buffer | ||
from compose import utils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason this is from compose
and not from ..
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, no reason, I can change it to be more consistent, but we do a bit of both in other modules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can normalise it all in another PR, I reckon
This will be so exciting to get this PR merged in. Aces |
LGTM |
1 similar comment
LGTM |
✨ 🎆 🗽 |
Oh! Great! Thanks @dnephin! Think this must've been the longest open PR, so glad to see this merged Does this also open the road to Windows? Or does that rely on other things? |
i might be feeling like my grandma when the wall in our city came down. (but there are arguments to consider that what followed as a mistake; Python 3 is not.) |
typo: fix typo in README.md
Progress on #219
The test suite passes, but I ran into some errors while trying to run it under python 3.4 manually (d11wtq/dockerpty#8 is the first).
It might be a good idea to have the test suite running python3 even if it doesn't work for real yet. That way new code will have to be compatible and it'll be less work to fix the remaining python3 issues (as we add tests which expose the problems).