listen on multiple addresses (and families)

listen on multiple addresses (and families)

[Andrew Svetlov, 2013-04-03 18:49:51]

Some comments

https://codereview.appspot.com/8307045/diff/1/tulip/base_events.py
File tulip/base_events.py (right):

https://codereview.appspot.com/8307045/diff/1/tulip/base_events.py#newcode455
tulip/base_events.py:455: reuse_addr = os.name == 'posix' and sys.platform !=
'cygwin'
Perhaps reuse_addr should be added to function signature with None default
value. 
If None — use heuristic like this line.
If True — force SO_REUSEADDR
If False — never set SO_REUSERADDR

Thoughs?

https://codereview.appspot.com/8307045/diff/1/tulip/base_events.py#newcode485
tulip/base_events.py:485: finally:
If I've understood correctly you close already created sockets if some family
has been already bound.
That's fine.
Should we raise more informative exception like:
cannot start serving {host}:{port} for {interface} since it's already allocated?

[Giampaolo Rodola, 2013-04-03 20:31:05]

https://codereview.appspot.com/8307045/diff/1/tulip/base_events.py
File tulip/base_events.py (right):

https://codereview.appspot.com/8307045/diff/1/tulip/base_events.py#newcode455
tulip/base_events.py:455: reuse_addr = os.name == 'posix' and sys.platform !=
'cygwin'
Agreed.

https://codereview.appspot.com/8307045/diff/1/tulip/base_events.py#newcode485
tulip/base_events.py:485: finally:
Agreed.

[Giampaolo Rodola, 2013-04-03 20:34:36]

I need to post an updated patch.
Can I somehow update this ticket or shall I create a brand new one?

[Nikolay Kim, 2013-04-03 20:41:31]

On 2013/04/03 20:34:36, g.rodola wrote:
> I need to post an updated patch.
> Can I somehow update this ticket or shall I create a brand new one?

upload.py -i 8307045

[Giampaolo Rodola, 2013-04-03 20:45:10]

Thanks, done.

2013/4/3  <fafhrd91@gmail.com>:
> On 2013/04/03 20:34:36, g.rodola wrote:
>>
>> I need to post an updated patch.
>> Can I somehow update this ticket or shall I create a brand new one?
>
>
> upload.py -i 8307045
>
> https://codereview.appspot.com/8307045/

[Nikolay Kim, 2013-04-03 20:55:44]

https://codereview.appspot.com/8307045/diff/2002/tests/events_test.py
File tests/events_test.py (right):

https://codereview.appspot.com/8307045/diff/2002/tests/events_test.py#newcode796
tests/events_test.py:796: # This is quite mysterious, but necessary.
this is probably "accept" and then "data_received"

i'd add test if server actually receiving b'xxx'

https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py
File tulip/base_events.py (right):

https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py#newcode469
tulip/base_events.py:469: type=socket.SOCK_STREAM, proto=0, flags=flags)
what if i want specific protocol?

[gvrpython, 2013-04-03 21:44:29]

Using the upload.py script, it's simple:

upload.py -i 8307045


On Wed, Apr 3, 2013 at 1:34 PM, <g.rodola@gmail.com> wrote:

> I need to post an updated patch.
> Can I somehow update this ticket or shall I create a brand new one?
>
>
https://codereview.appspot.**com/8307045/<https://codereview.appspot.com/8307...
>



-- 
--Guido van Rossum (python.org/~guido)

[Giampaolo Rodola, 2013-04-03 22:55:17]

https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py
File tulip/base_events.py (right):

https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py#newcode469
tulip/base_events.py:469: type=socket.SOCK_STREAM, proto=0, flags=flags)
On 2013/04/03 20:55:44, Nikolay Kim wrote:
> what if i want specific protocol?

Are there cases where you actually want to do that?

[Nikolay Kim, 2013-04-03 23:02:38]

On 2013/04/03 22:55:17, g.rodola wrote:
> https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py
> File tulip/base_events.py (right):
> 
>
https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py#newcode469
> tulip/base_events.py:469: type=socket.SOCK_STREAM, proto=0, flags=flags)
> On 2013/04/03 20:55:44, Nikolay Kim wrote:
> > what if i want specific protocol?
> 
> Are there cases where you actually want to do that?

ah. i didnt notice you removed proto from start_serving.

i think you should use "*" after port=None for start_serving.

[Giampaolo Rodola, 2013-04-03 23:34:54]

On 2013/04/03 23:02:38, Nikolay Kim wrote:
> On 2013/04/03 22:55:17, g.rodola wrote:
> > https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py
> > File tulip/base_events.py (right):
> > 
> >
>
https://codereview.appspot.com/8307045/diff/2002/tulip/base_events.py#newcode469
> > tulip/base_events.py:469: type=socket.SOCK_STREAM, proto=0, flags=flags)
> > On 2013/04/03 20:55:44, Nikolay Kim wrote:
> > > what if i want specific protocol?
> > 
> > Are there cases where you actually want to do that?
> 
> ah. i didnt notice you removed proto from start_serving.
> 
> i think you should use "*" after port=None for start_serving.

done

[Nikolay Kim, 2013-04-03 23:53:10]

You should change AbstractEventLoop.start_serving signature
to reflect new api. I'd move doc string as well.

otherwise LGTM.

[GvR, 2013-04-23 03:29:55]

There's a start_serving() call in srv.py that will break with this change. 
Possibly other calls in other examples recently added too.  Please fix those. 
Also one comment about docstring markup.

https://codereview.appspot.com/8307045/diff/22001/tulip/events.py
File tulip/events.py (right):

https://codereview.appspot.com/8307045/diff/22001/tulip/events.py#newcode188
tulip/events.py:188: """Creates a TCP server bound to *host* and *port* and
return
Please don't use markup like *host* to refer to the arguments.  It sounds a bit
like *shouting* to my ears.