Is a UDP transport also a ReadTransport/WriteTransport?
| BPO | 31176 |
|---|---|
| Nosy | @1st1, @twisteroidambassador |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2017-08-10.13:28:01.796>
labels = ['type-feature', '3.7', 'docs', 'expert-asyncio']
title = 'Is a UDP transport also a ReadTransport/WriteTransport?'
updated_at = <Date 2017-08-10.13:28:01.796>
user = 'https://github.com/twisteroidambassador'
bugs.python.org fields:
activity = <Date 2017-08-10.13:28:01.796>
actor = 'twisteroid ambassador'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation', 'asyncio']
creation = <Date 2017-08-10.13:28:01.796>
creator = 'twisteroid ambassador'
dependencies = []
files = []
hgrepos = []
issue_num = 31176
keywords = []
message_count = 1.0
messages = ['300083']
nosy_count = 3.0
nosy_names = ['docs@python', 'yselivanov', 'twisteroid ambassador']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31176'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']
In docs / Library Reference / asyncio / Transports and Protocols, it is mentioned that "asyncio currently implements transports for TCP, UDP, SSL, and subprocess pipes. The methods available on a transport depend on the transport’s kind." It also lists methods available on a BaseTransport, ReadTransport, WriteTransport, DatagramTransport and BaseSubprocessTransport.
However, the docs does not explain which transports have methods from which base classes, or in other words which base classes each concrete transport class inherits from. And this may not be obvious: for example, a UDP transport certainly is a DatagramTransport, but is it also a ReadTransport, or a WriteTransport?
(I feel like the answer is "no it isn't", but there are plenty of conflicting evidence. The docs show that WriteTransport has write_eof() and can_write_eof() -- methods clearly geared towards stream-like transports, and it duplicates abort() from DatagramTransport, so it would seem like WriteTransport and DatagramTransport are mutually exclusive. On the other hand, the default concrete implementation of _SelectorDatagramTransport actually inherits from Transport which inherits from both ReadTransport and WriteTransport, yet it does not inherit from DatagramTransport; As a result _SelectorDatagramTransport has all the methods from ReadTransport and WriteTransport, but many of them raise NotImplemented. This is why I'm asking this question in the first place: I found that the transport object I got from create_datagram_endpoint() has both pause_reading() and resume_reading() methods that raise NotImplemented, and thought that perhaps some event loop implementations would have these methods working, and I should try to use them. And before you say "UDP doesn't do flow control", asyncio actually does provide flow control for UDP on the writing end: see https://www.mail-archive.com/[email protected]/msg00532.html So it's not preposterous that there might be flow control on the reading end as well.)
I think it would be nice if the documentation can state the methods implemented for each type of transport, as the designers of Python intended, so there's a clear expectation of what methods will / should be available across different implementations of event loops and transports. Something along the lines of "The methods available on a transport depend on the transport’s kind: TCP transports support methods declared in BaseTransport, ReadTransport and WriteTransport below, etc."
@CAM-Gerlach Do you know if this inheritance pattern can be expressed better with a sphinx extension or something or does this needs to be done manually?
We should start by collecting the actual graph of inheritance between the various transports, here in this issue.
In terms of support for presenting this information, there's the built in e.g. sphinx.ext.inheritance_diagram. But it seems to me, as it does @gvanrossum, that the main task at present is determining the intended and/or actual inheritance pattern between these classes. Once that is in hand, we can decide how to best present it, and implement that in the docs (which I could help with if desired).
In terms of support for presenting this information, there's the built in e.g. sphinx.ext.inheritance_diagram.
Thanks, I didn't know that sphinx has graphviz support.
There is one bad news, the current inheritance of datagram transports is broken (I remembered that this was fixed long ago but apparently not). See https://github.com/python/cpython/issues/90352 by @asvetlov
On Windows, the proactor datagram transport is implemented correctly and has the following inheritance pattern:
classDiagram
BaseTransport <|-- DatagramTransport
BaseTransport <|-- ReadTransport
BaseTransport <|-- WriteTransport
BaseTransport <|-- _ProactorBasePipeTransport
DatagramTransport <|-- _ProactorDatagramTransport
ReadTransport <|-- Transport
Transport <|-- _FlowControlMixin
WriteTransport <|-- Transport
_FlowControlMixin <|-- _ProactorBasePipeTransport
_ProactorBasePipeTransport <|-- _ProactorDatagramTransport
object <|-- BaseTransport
Can you post a drawing for the selector datagram transport? Also maybe we should just merge the PR that supposedly fixes the thing? (Can you review it?)
Selector Datagram Transport Inheritance:
classDiagram
BaseTransport <|-- ReadTransport
BaseTransport <|-- WriteTransport
ReadTransport <|-- Transport
Transport <|-- _FlowControlMixin
Transport <|-- _SelectorTransport
WriteTransport <|-- Transport
_FlowControlMixin <|-- _SelectorTransport
_SelectorTransport <|-- _SelectorDatagramTransport
object <|-- BaseTransport
(Can you review it?)
The PR has merge conflicts and CI was failing at the last run so I can't review it.
In that case, can you come up with your own PR (maybe inspired by that one) that does the intended thing?
Before doing that I have a question what inheritance do we want for _SelectorDatagramTransport? Currently it inherits from Transport which IMO is wrong since it has methods like write which raise NotImplementedError, it should only inherit from things which it actually implements.
Also it seems this inheritance issue is from the beginning as asyncio was imported from the old repo https://github.com/python/cpython/blame/b27b57c6e44a276c8a9843fd37d4cf65b2827d5c/Lib/asyncio/selector_events.py#L1129. So you would probably know why things are implemented like this rather than how I would like it to be.
Before doing that I have a question what inheritance do we want for
selector DatagramTransport? Currently it inherits fromTransportwhich IMO is wrong since it has methods likewritewhich raiseNotImplementedError, it should only inherit from things which it actually implements.
Yeah, that would be nice but we're constrained by where some of the implementation bits are. We'd have to introduce even more intermediate classes in the class hierarchy.
Maybe all we need is to add DatagramTransport to the bases of _SelectorDatagramTransport (last in the list of base classes, so its abstract abort() doesn't override the concrete implementation in _SelectorTransport).
PS. The diagram for _ProactorDatagramTransport is confusing, it shows DatagramTransport first, but it should be last (for similar reasons).
Maybe all we need is to add DatagramTransport to the bases of _SelectorDatagramTransport (last in the list of base classes, so its abstract abort() doesn't override the concrete implementation in _SelectorTransport).
https://github.com/python/cpython/pull/98844 adds DatagramTransport to the bases of _SelectorDatagramTransport.
It seems the initial issue here was one with the documentation rather than the implementation, specifically:
the docs does not explain which transports have methods from which base classes, or in other words which base classes each concrete transport class inherits from
An issue with the implementation of the inheritance hierarchy (#90352) that was brought up along the way was fixed, but has the documentation issue here actually been addressed, i.e. by documenting what that inheritance hierarchy currently is (e.g. via an inheritance diagram)?