Issue 23630: support multiple hosts in create_server/start_server (original) (raw)
Created on 2015-03-10 18:18 by sebastien.bourdeauducq, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Messages (43)
Author: Sebastien Bourdeauducq (sebastien.bourdeauducq) *
Date: 2015-03-10 18:18
I would like to be able to bind asyncio TCP servers to an arbitrary list of hosts, not just either one host or all interfaces.
I propose that the host parameter of create_server and start_server can be a list of strings that describes each host. Sockets are created for the set of all addresses of all specified hosts. The list may also contain None, or the empty string, in which case all interfaces are assumed.
If a string or None is passed directly, the behavior is unchanged - so this should not break compatibility.
I can submit a patch if this feature is approved.
Author: Guido van Rossum (gvanrossum) * 
Date: 2015-03-10 18:27
This makes some sense, but it's easy to work around -- just call create_server() multiple times, once for each host. Why does that not work for you?
Author: Sebastien Bourdeauducq (sebastien.bourdeauducq) *
Date: 2015-03-10 18:45
That could work, but I would have to manually resolve and filter the lists of hostnames so that the program does not attempt to bind the same address twice - something that is better implemented in create_server, which already does the hostname resolution. Additionally, the asyncio Server internally supports listening on multiple sockets, and that would be the right feature to use for implementing this instead of creating and managing multiple servers.
Author: Guido van Rossum (gvanrossum) *
Date: 2015-03-10 18:46
So it sounds like you already have a patch in mind... Can you work on it and upload it? (It's open source -- you get to make it yourself. :-)
Author: Sebastien Bourdeauducq (sebastien.bourdeauducq) *
Date: 2015-03-10 19:20
See attached.
Author: Guido van Rossum (gvanrossum) *
Date: 2015-03-10 19:28
Could use a test. Hopefully Victor can do a thorough review.
On Tue, Mar 10, 2015 at 12:20 PM, Sebastien Bourdeauducq < report@bugs.python.org> wrote:
Sebastien Bourdeauducq added the comment:
See attached.
keywords: +patch Added file: http://bugs.python.org/file38428/asyncio_multibind.diff
Python tracker <report@bugs.python.org> <http://bugs.python.org/issue23630>
Author: Yann Sionneau (ysionneau) *
Date: 2015-03-11 13:07
Here is a test related to Sebastien's patch.
Author: STINNER Victor (vstinner) *
Date: 2015-03-11 13:18
Combine patch written for CPython tree, so we get the "review" button.
Author: Yann Sionneau (ysionneau) *
Date: 2015-03-20 10:46
Please let me know if you need anything for this patch to be integrated.
Author: Yann Sionneau (ysionneau) *
Date: 2015-04-21 08:38
ping ?
Author: STINNER Victor (vstinner) *
Date: 2015-07-01 06:35
I reviewed multibind.patch.
Author: Yann Sionneau (ysionneau) *
Date: 2015-08-26 20:41
Here is a new patch, with modifications according to your review Victor. Thanks!
Author: Yury Selivanov (yselivanov) *
Date: 2015-08-26 20:47
Here is a new patch, with modifications according to your review Victor.
Could you please generate the patch with 'hg diff', so that code review works with it.
Author: Yann Sionneau (ysionneau) *
Date: 2015-08-26 21:02
Ah yes, sorry about that. Here is a new patch. However, I worked on these sources: https://github.com/python/asyncio which now I understand was not the correct repository, but I added a tests_require="netifaces" in the setup.py, where can I add this dependency in the CPython repository architecture? Thanks!
Author: Yury Selivanov (yselivanov) *
Date: 2015-08-26 22:35
Left some feedback in the code review.
I have a question about the api: so we allow to pass a set of hosts to create_server, but why do we have only a single port?
Author: Yann Sionneau (ysionneau) *
Date: 2015-08-27 16:43
All right, thanks. I fixed everything but the thing about getaddrinfo being Mock'ed. How am I supposed to handle the method being called with a Mock'ed getaddrinfo?
Author: Yury Selivanov (yselivanov) *
Date: 2015-08-27 16:48
How am I supposed to handle the method being called with a Mock'ed getaddrinfo?
Why would you call it that way? Are regular asyncio users going to mock it in their production/test code?
If the only case for using mocks there are asyncio unittests -- just don't do that.
Author: Yann Sionneau (ysionneau) *
Date: 2015-08-27 16:56
All right, here is the new patch then.
Author: Yury Selivanov (yselivanov) *
Date: 2015-08-27 17:24
Thanks!
The only remaining question is is it OK that we can specify more than one host but only one port? Guido, Victor?
Author: Guido van Rossum (gvanrossum) *
Date: 2015-08-27 18:01
Yeah, I think this is fine. I think the use case for multiple hosts is very different than that for multiple ports, so I see no reason to link the two features, and multiple ports hasn't been requested.
The patch has a long line and the new docstring looks a little odd (sentences not starting with capital letters) but otherwise I'm in favor of this going into 3.6.
Author: Yury Selivanov (yselivanov) *
Date: 2015-08-27 19:00
Yeah, I think this is fine. I think the use case for multiple hosts is very different than that for multiple ports, so I see no reason to link the two features, and multiple ports hasn't been requested.
OK!
The patch has a long line and the new docstring looks a little odd (sentences not starting with capital letters)
Yeah, I've noticed that too, and made a mental note to check line lengths before committing.
but otherwise I'm in favor of this going into 3.6.
Why only 3.6? The change is fully backwards compatible, why can't it go in 3.5.1?
Also, what's the protocol for committing something that might take years to be available in github/asyncio?
Author: Guido van Rossum (gvanrossum) *
Date: 2015-08-27 19:03
It can't go into 3.5.1 (a bugfix release) because it is a new feature (however small), and asyncio is not provisional in 3.5 (only in 3.4).
Author: Guido van Rossum (gvanrossum) *
Date: 2015-08-27 19:04
Also, I guess you'll need a branch that tracks what's going into 3.6.
Author: Yann Sionneau (ysionneau) *
Date: 2015-08-27 19:05
About the missing capitalization, I omitted it because it's the argument's name. But I can resubmit with it being capitalized. I forgot to run flake8, sorry!
Author: Guido van Rossum (gvanrossum) *
Date: 2015-08-27 19:07
The traditional solution is to rewrite the sentence so the name is not the first word. :-)
Author: Yann Sionneau (ysionneau) *
Date: 2015-08-28 14:06
A new patch with capitalization of first words of sentences + splitting lines which are too long (more PEP8 friendly).
Author: STINNER Victor (vstinner) *
Date: 2015-09-02 14:07
The only remaining question is is it OK that we can specify more than one host but only one port? Guido, Victor?
I can be convenient to have a single Server object with all listening sockets. So I like the idea of supporting multiple ports. It's very cheap to support it.
Author: STINNER Victor (vstinner) *
Date: 2015-09-02 14:19
Since the consensus on python-dev looks more to keep the provisional API for asyncion in Python 3.5, I propose to even modify Python 3.4 to not add too many "if python >= 3.5..." checks in asyncio source code.
If you really prefer to keep Python 3.4 unchanged, we should at least add the feature in Python 3.5.1, having to wait for Python 3.6 is too long, no?
Author: Yury Selivanov (yselivanov) *
Date: 2015-09-02 14:27
I can be convenient to have a single Server object with all listening sockets. So I like the idea of supporting multiple ports. It's very cheap to support it.
I like the idea too, but I'm not sure how to redesign the function's signature to accept multiple hosts & ports. We have a separate param for 'port', and this patch allows to pass an array of hosts in 'host' argument. Should we allow passing an array of (host, port) tuples in 'host' with 'None' in 'port'? This would be one ugly method...
Since the consensus on python-dev looks more to keep the provisional API for asyncion in Python 3.5, I propose to even modify Python 3.4 to not add too many "if python >= 3.5..." checks in asyncio source code.
asyncio is provisional in 3.4 (and now in 3.5 too), so we can just keep updating it in bugfix releases.
If you really prefer to keep Python 3.4 unchanged, we should at least add the feature in Python 3.5.1, having to wait for Python 3.6 is too long, no?
This was before we reinstated asyncio as provisional in 3.5.
But before committing this change, let's agree if we want this API to accept multiple ports when you specify more than one host.
We also need to create a dev branch on github/asyncio, so that it's easier for us to sync PyPI package with CPython releases.
Author: STINNER Victor (vstinner) *
Date: 2015-09-02 14:28
"So I like the idea of supporting multiple ports. It's very cheap to support it."
Oh, I had an example in my head: listening on tcp/80 for HTTP and tcp/443 for HTTPS... but it doesn't work. The problem is that the proposed API doesn't allow to specify a protocol factory per port. For such use case, you have to handle multiple servers.
Anyway, I still like the idea of listening to multiple hosts and/or multiple ports. It's a common requirement to start a server.
Author: Yann Sionneau (ysionneau) *
Date: 2015-09-02 14:31
About the function's signature to accept multiple hosts & ports, we could
- accept host and port arguments being sequences and then we bind to all host:port combinations. Like if len(host) == N and len(port) == M, we bind to N*M sockets.
or
- accept host and port arguments being sequences and allow repetitions and bind to host:port couples from zip(host, port).
I think the first is "clean", but the second allows more flexibility. Like for instance binding to IP1:port1 IP2:port2 but not IP1:port2
Author: Guido van Rossum (gvanrossum) *
Date: 2015-09-02 14:35
Let's not extend the API with support for multiple ports. Nobody has asked for it, and clearly the API design is not so simple.
Author: Yury Selivanov (yselivanov) *
Date: 2015-09-02 14:38
- accept host and port arguments being sequences and then we bind to all host:port combinations. Like if len(host) == N and len(port) == M, we bind to N*M sockets.
I can't think of a real life example for this. But if you need that, you can always implement that on top of function that accepts multiple (host,port) pairs.
- accept host and port arguments being sequences and allow repetitions and bind to host:port couples from zip(host, port).
I'm not sure I like this either. This breaks one value (h,p) into two values, and passes them in separate collections to separated arguments. Too complex.
My proposal is to allow the following:
create_server(protofac, [(h1,p1),(h2,p2)])
Author: STINNER Victor (vstinner) *
Date: 2015-09-02 14:41
Let's not extend the API with support for multiple ports. Nobody has asked for it, and clearly the API design is not so simple.
Ok. Anyway, the user of the API call call create_server() multiple times, even with the current code. So it's fine.
Author: Yury Selivanov (yselivanov) *
Date: 2015-09-02 16:26
Yann, I'll commit this as soon as 3.5.0 and 3.4.4 are out.
Author: Yann Sionneau (ysionneau) *
Date: 2015-09-03 15:16
Here is a new patch without any dependency on netifaces. Thanks Victor for the great deal of help for all the mock() stuff!
Author: Yann Sionneau (ysionneau) *
Date: 2015-09-04 14:19
A new (and hopefully last?) version of the patch, thanks again for the review Victor!
Author: STINNER Victor (vstinner) *
Date: 2015-09-04 15:19
Good. I have less comments, it's getting better at each iteration ;)
Author: Yann Sionneau (ysionneau) *
Date: 2015-09-04 22:49
Yet another version of the patch according to the last review.
Author: Roundup Robot (python-dev) 
Date: 2015-09-21 16:42
New changeset 682623e3e793 by Victor Stinner in branch '3.4': Issue #23630, asyncio: host parameter of loop.create_server() can now be a https://hg.python.org/cpython/rev/682623e3e793
Author: STINNER Victor (vstinner) *
Date: 2015-09-21 16:45
I pushed the change to Python 3.4, 3.5, 3.6 and asyncio at Github.
Thanks Yann for your nice enhancement of asyncio! It put your name in CPython "acks" and asyncio author list.
I modified the patch a little bit (3 lines), how the hosts variable was initialized. That's all :-)
Author: Roundup Robot (python-dev)
Date: 2015-09-21 20:32
New changeset 42e7334e0e4c by Victor Stinner in branch '3.4': Issue #23630: Fix test_asyncio on Windows https://hg.python.org/cpython/rev/42e7334e0e4c
New changeset 840942a3f6aa by Victor Stinner in branch '3.5': Merge 3.4 (Issue #23630, fix test_asyncio) https://hg.python.org/cpython/rev/840942a3f6aa
New changeset 1f979a573f8d by Victor Stinner in branch 'default': Merge 3.5 (Issue #23630, fix test_asyncio) https://hg.python.org/cpython/rev/1f979a573f8d
Author: Yann Sionneau (ysionneau) *
Date: 2015-09-22 22:06
Thanks a lot Victor for your numerous reviews and your help on this! Also thanks for all other who commented.
History
Date
User
Action
Args
2022-04-11 14:58:13
admin
set
github: 67818
2015-09-22 22:06:25
ysionneau
set
messages: +
2015-09-21 20:32:28
python-dev
set
messages: +
2015-09-21 16:45:49
vstinner
set
status: open -> closed
resolution: fixed
messages: +
2015-09-21 16:42:42
python-dev
set
nosy: + python-dev
messages: +
2015-09-04 22:49:18
ysionneau
set
files: + multibind8.patch
messages: +
2015-09-04 15:19:38
vstinner
set
messages: +
2015-09-04 14:19:20
ysionneau
set
files: + multibind7.patch
messages: +
2015-09-03 15:16:45
ysionneau
set
files: + multibind6.patch
messages: +
2015-09-02 16:26:26
yselivanov
set
messages: +
2015-09-02 14:41:03
vstinner
set
messages: +
2015-09-02 14:38:28
yselivanov
set
messages: +
2015-09-02 14:35:27
gvanrossum
set
messages: +
2015-09-02 14:31:58
ysionneau
set
messages: +
2015-09-02 14:28:05
vstinner
set
messages: +
2015-09-02 14:27:42
yselivanov
set
messages: +
2015-09-02 14:19:11
vstinner
set
messages: +
2015-09-02 14:07:48
vstinner
set
messages: +
2015-08-28 14:06:12
ysionneau
set
files: + multibind4.patch
messages: +
2015-08-27 19:07:10
gvanrossum
set
messages: +
2015-08-27 19:05:47
ysionneau
set
messages: +
2015-08-27 19:04:30
gvanrossum
set
messages: +
2015-08-27 19:03:57
gvanrossum
set
messages: +
2015-08-27 19:00:17
yselivanov
set
messages: +
2015-08-27 18:01:21
gvanrossum
set
messages: +
2015-08-27 17:24:06
yselivanov
set
messages: +
2015-08-27 16:56:37
ysionneau
set
files: + multibind3.patch
messages: +
2015-08-27 16:48:41
yselivanov
set
messages: +
2015-08-27 16:43:40
ysionneau
set
messages: +
2015-08-26 22:35:46
yselivanov
set
messages: +
2015-08-26 21:02:50
ysionneau
set
files: + multibind2.patch
messages: +
2015-08-26 20:58:11
ysionneau
set
files: - multibind2.patch
2015-08-26 20:47:38
yselivanov
set
messages: +
2015-08-26 20:41:53
ysionneau
set
files: + multibind2.patch
messages: +
2015-07-01 06:35:20
vstinner
set
messages: +
2015-04-21 08:38:11
ysionneau
set
messages: +
2015-03-20 10:46:35
ysionneau
set
messages: +
2015-03-11 13🔞28
vstinner
set
files: + multibind.patch
messages: +
2015-03-11 13:07:15
ysionneau
set
files: + asyncio_multibind_test.diff
nosy: + ysionneau
messages: +
2015-03-10 19:28:35
gvanrossum
set
messages: +
2015-03-10 19:20:56
sebastien.bourdeauducq
set
files: + asyncio_multibind.diff
keywords: + patch
messages: +
2015-03-10 18:46:55
gvanrossum
set
messages: +
2015-03-10 18:45:02
sebastien.bourdeauducq
set
messages: +
2015-03-10 18:27:09
gvanrossum
set
messages: +
2015-03-10 18🔞41
sebastien.bourdeauducq
create