Provide an API to handle connections that are accepted outside of ayncio #378
Conversation
|
I don't like the name -- how about |
|
I think the name should be discussed over here: https://bugs.python.org/issue27392. |
|
What's wrong with the Python 3.4 tests on Windows (AppVeyor)? Can we share more code between this and _create_connection_transport()? I'll deliberate the name in the Python tracker issue. |
|
I have no idea WRT Python 3.4 on windows. It seems to be hanging. I don't have a windows development environment. Do you by any chance know where I can find a good howto for setting one up? |
|
Just a note WRT the name: Really it's about more than the name. @1st1 argues that there should be a new API for wrapping already-connected sockets, for both clients and servers, and that create_connection shouldn't accept connected sockets. I think this is a pretty good argument. |
|
WRT sharing code, I guess I could make |
|
OK, more sharing. Maybe I'll be lucky and the windows tests will pass. :) |
|
It's also about more than finding the perfect API. (Not that everyone would
even agree on that.)
There are several APIs that create and configure a socket in a certain way
and wrap it in a transport/protocol pair, and all of them have the option
of taking an existing socket. I think that's a reasonable model and we
should not abandon it.
The complication we have here is that there are two types of server
sockets, the "listening" kind and the "connected" kind. If you're really
into finding the perfect API, maybe the right thing to do would be to
search for a way to refactor create_server()?
|
In hopes of seeing wtf failure on windows.
| @@ -980,6 +981,25 @@ def create_server(self, protocol_factory, host=None, port=None, | |||
| return server | |||
|
|
|||
| @coroutine | |||
| def connect_accepted_socket(self, protocol_factory, sock, ssl=None): | |||
There was a problem hiding this comment.
Let's make ssl a keyword-only argument (as it is in create_connection and create_server).
|
Aside from a couple of nits it looks good. |
|
The tests hanging for the ProactorEventLoop with SSL. I'll skip that case. BTW, running tests verbosely is helpful for cases like this. Is there any harm in always running the appveyor tests verbosely? |
Strange, sounds like we need to open an issue for this (but it shouldn't block this PR).
Yes, it makes sense. Can you make a separate PR to run tests in verbose mode both for Travis and AppVeyor? |
No, It's documented:
Jim Jim Fulton |
|
OK, all tests passing. Assuming this is merged, I'll update the docs. Where do I do that? |
|
LGTM. Will merge it in a few hours.
@gvanrossum Should we copy asyncio docs from CPython to this repo and update the |
|
Regarding the docs: that would only be useful if we also had the machinery
to build the docs here.
I think it may be time actually to give up on this github repo and focus
all development in the CPython repo. Especially since I also think that
asyncio should drop the "provisional" status in Python 3.6.
Maybe we could keep this repo for something called "backports/asyncio",
more in line with the backports versions that exist of other Python 3
packages (e.g. enum34). That might also make it easier to distribute an
alternative version that can override the stdlib version.
|
|
Can this be merged? If it is, will it be in 3.6? I'm happy to update the docs wherever. |
Merged. Yes, it will be in 3.6 (and I'll add it to uvloop in a couple of days). For the docs please upload a patch to https://bugs.python.org/issue27392.
Please let's wait until CPython migrates to GitHub. I like code review workflow here much better. |
This addresses: https://bugs.python.org/issue27392