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

Attach method using websockets #62

Merged
merged 4 commits into from
Nov 8, 2013
Merged

Conversation

aanand
Copy link
Contributor

@aanand aanand commented Oct 3, 2013

client.attach_websocket() works just like client.attach_socket(), but uses the /attach/ws endpoint and connects using the websocket Python library.

@shin-
Copy link
Contributor

shin- commented Oct 14, 2013

Why not have a ws=Falseparameter in attach_socket instead? I don't think this warrants adding a new method.

If you can change this (and rebase the PR), I'd be more than happy to accept it.

@aanand
Copy link
Contributor Author

aanand commented Oct 14, 2013

I'd argue that it's worth adding a new method for the same reason that the HTTP and Websockets endpoints have different URLs: methods where the returned value can be one of two completely different types are a bit weird.

But I don't have strong feelings either way, so I'm happy to make that change (some time in the next day or two, probably).

@shin-
Copy link
Contributor

shin- commented Nov 7, 2013

Cool, thanks for the update! I'll try to merge it by the end of this week.

@shin-
Copy link
Contributor

shin- commented Nov 8, 2013

So it looks like websocket-client doesn't support python 3 yet. I'm putting six.PY3 guards where it's used to avoid breaking the tests, and we'll have to watch for support in a future version (looks like they're actively working on it)

@shin- shin- merged commit bd938b5 into docker:master Nov 8, 2013
@shin-
Copy link
Contributor

shin- commented Nov 8, 2013

Thanks again for the contribution :)

@aanand
Copy link
Contributor Author

aanand commented Nov 8, 2013

Whoops! Good call, forgot to test it on 3.

On Fri, Nov 8, 2013 at 1:13 PM, Joffrey F [email protected] wrote:

Thanks again for the contribution :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/62#issuecomment-28084568
.

@aanand aanand deleted the attach-websocket branch January 16, 2014 17:08
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