Add new TCPTransport base class #286

Activity

1. 1st1

   changed the title [-]Transport.get_extra_info[/-] [+]Add new TCPTransport base class[/+] on Nov 12, 2015
2. vstinner commented on Nov 12, 2015

   

   vstinner

   on Nov 12, 2015

   Member

   More actions

   "There is actually no way to access the underlying socket, which causes some problems. "

   Hum, how do you plan to set TCP_NODELAY flag if you don't have access to the socket?

   There are a lot of socket flags and options. I'm not sure that it's a good idea to add a method for each flag. It will be difficult to document the available of each method depending on the platform and the implementation of the event loop. We already have to take care in the documentation of subtle differences between SelectorEventLoop on UNIX and on Windows, and the special case of ProactorEventLoop.
3. asvetlov commented on Nov 12, 2015

   

   asvetlov

   on Nov 12, 2015

   More actions

   Thinking on the issue I would suggest another solution: don't export 'socket' extra info by uvloop (it's impossible) but add uv-specific slot for controlling socket options in libuv way.

   E.g. for utilizing nodelay and keepalive in aiohttp I should check for available options anyway: old asyncio should be supported as well as other asyncio-compatible loops like quamash etc. Explicit check for 'uv' along with 'socket' don't require a big maintenance burden, reusing existing API looks attractive.
4. vstinner commented on Nov 12, 2015

   

   vstinner

   on Nov 12, 2015

   Member

   More actions

   "Thinking on the issue I would suggest another solution: don't export 'socket' extra info by uvloop (it's impossible) (...)"

   Hum, does it mean that we should update the doc to mention that some info may lack depending on the event loop?

   https://docs.python.org/dev/library/asyncio-protocol.html#asyncio.BaseTransport.get_extra_info
5. asvetlov commented on Nov 12, 2015

   

   asvetlov

   on Nov 12, 2015

   More actions

   I believe yes.
   If some transport implementation cannot provide 'socket' extra info it cannot be documented as mandatory.
   it makes less harm than stating that libuv cannot be used with asyncio at all.
6. 1st1 commented on Nov 12, 2015

   

   1st1

   on Nov 12, 2015

   MemberAuthor

   More actions

   Hum, how do you plan to set TCP_NODELAY flag if you don't have access to the socket?

   Libuv has two API methods for setting NODELAY and KEEPALIVE.

   There are a lot of socket flags and options.

   These two are important and popular enough to have explicit methods in Twisted and Tornado.

   I'm not sure that it's a good idea to add a method for each flag.

   I agree. But in this specific case - those are very popular and important options. And I simply can't make my loop usable without them.

   Hum, does it mean that we should update the doc to mention that some info may lack depending on the event loop?

   Absolutely. get_extra_info(..) may return None and you have to expect that.
7. gvanrossum commented on Nov 12, 2015

   

   gvanrossum

   on Nov 12, 2015

   Member

   More actions

   I like Andrew's suggestion. Expose the libuv stream interface (wrapped in a helper object) through get_extra_info('libuv-stream'). That avoids the need for modifying the class structure of asyncio (or any other part of its implementation or interface). So user code could look like e.g.

   uvstr = t.get_extra_info('libuv-stream')
   if uvstr is not None:
       uvstr.set_nodelay(True)
   else:
       sock = t.get_extra_info('socket')
       if sock is not None:
           sock.setsockopt(...)
       else:
           print('Cannot set nodelay')
   

   Depending on how popular uvloop becomes we could eventually add a standard interface to asyncio so that you wouldn't have to do it one way for regular sockets and another way for uvloop, but that's not so important in the short term.
8. 1st1 commented on Nov 25, 2015

   

   1st1

   on Nov 25, 2015

   MemberAuthor

   More actions

   To my surprise (I think the function is documented in a wrong place) I found a way to extract the fileno from a libuv stream. Now I can use socket.fromfd to finally implement get_extra_info('socket').

   Still, I like the idea of explicit set_keepalive and set_nodelay. To me get_extra_info is just a leaking abstraction. We should provide a better APIs for common tasks.
9. gvanrossum commented on Nov 25, 2015

   

   gvanrossum

   on Nov 25, 2015

   Member

   More actions

   Hum. At what level in a network stack do "keepalive" and "nodelay" really
   belong? I honestly don't even know what they mean. I still think that
   people who want to set such options need to understand that they are
   breaking through the abstractions provided by asyncio, and get_extra_info()
   is exactly the right API to clarify that.

   On Wed, Nov 25, 2015 at 9:08 AM, Yury Selivanov notifications@github.com
   wrote:

   To my surprise (I think the function is documented in a wrong place) I
   found a way http://docs.libuv.org/en/v1.x/handle.html#c.uv_fileno to
   extract the fileno from a libuv stream. Now I can use socket.fromfd to
   finally implement get_extra_info('socket').

   Still, I like the idea of explicit set_keepalive and set_nodelay. To me
   get_extra_info is just a leaking abstraction. We should provide a better
   APIs for common tasks.

   —
   Reply to this email directly or view it on GitHub
   #286 (comment).

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

    closed this as completedon Nov 8, 2016