Yury Selivanov <yseliva...@gmail.com> added the comment:
Nathaniel, thanks for the summary! A few comments to address some points: > So Yury and I are tentatively thinking we'll make some kind of PEP for the > 3.9 timescale, probably just for the core ABCs Yes! At the very least for things like asynchronous version of "closable", e.g. an object with an "aclose()" coroutine method. I'm sure there are some other straightforward design patterns that we can codify. Maybe we can do that for streams too -- see some thoughts below. > First decision point: will asyncio.Stream implement the ABCs directly, or > will you need some kind of adapter? I'd love asyncio.Stream to implement the ABCs directly. The only problem is that Trio isn't yet settled on the design of those ABCs and we need to make some decisions for asyncio *now*. I hope that the Trio project can minimize the number of methods they want to add to those ABCs so that we don't need to duplicate a lot of functionality in asyncio.Stream. E.g. in the new asyncio.Stream there's a Stream.write() coroutine; in Trio it's Stream.send_all(). I'm not entirely convinced that "send_all()" is a good name, for example, even though I now understand the motivation. We can discuss that later in a relevant issue though. Another important point to consider: if the new Trio Stream ABCs are *significantly different* from asyncio.Stream and would require us to alias too many methods or to do heavy refactoring and deprecations, then Trio will have to show some real world usage and traction of its APIs first. > If we completely separated the old StreamReader/StreamWriter functions from > the new Stream functions, then it would also have another advantage: we could > clean up several issues with Stream that are only needed for compatibility > with the old APIs. In particular, we could get rid of the optional-await > hacks on 'write' and 'close', and turn them into regular coroutines. We'd like to avoid that and have one asyncio.Stream class in asyncio. Using legacy StreamReader/StreamWriter functions for subprocesses alone (long term) isn't a solution for us, since there're real problems with .write() and .close() not being awaitables. Sticking to having a new asyncio.Stream API and old StreamReader/StreamWriter for subprocesses isn't an acceptable solution either. We'd like to minimize the API surface that asyncio users have to deal with. > The obvious change would be to leave out __aiter__ from asyncio.Stream in 3.8. > Option 1: Keep the Stream code as-is, and accept that using asyncio.Stream > with future ABC-based code will require some compromises Nathaniel, I think it's important to emphasize that those compromises should be mutual. I'm willing to support changing "Stream.close()" to "Stream.aclose()" and to perhaps alias some methods. We can also implement "Stream.chunks()". But changing the semantics of "__aiter__" is, unfortunately, not on the table, at least for me. If Trio doesn't want to change the __aiter__ semantics of its Stream ABC (which is only a *proposal* right now!), then: - Fragmenting asyncio APIs by letting subprocesses use old StreamReader/StreamWriter while we have new asyncio.Stream isn't an option. - Asking us to implement new subprocess APIs just for the sake of having different Stream implementation for Process.std* channels isn't an option either. Adding new APIs and deprecating old ones is a huge burden on asyncio maintainers and users. So the "obvious change" for *me* would be using "Stream.chunks()" iterator in Trio. For Trio it's a question of whether the new API is pretty; for asyncio it's a question of how many APIs we need to deprecate/change. I hope you understand my position and why I am strong -1 on the "Option 2". > Right now we don't have any path to fixing 'write'/'close' at all; Andrew and I discussed that this morning. Here's the plan: 1. We add "aclose()" and "write()" coroutines to the new "asyncio.Stream()". It won't have "wait_closed()" or "drain()" or "close()". 2. We add a _LegacyStream class derived from the new asyncio.Stream. We will use it for subprocesses. Its "write()" method will return an "OptionallyAwaitable" wrapper that will nudge users to add an await in front of "stdin.write()". _LegacyStream will be completely backwards compatible. This path enables us to add a decent new streaming API while retaining consistency and backwards compatibility. > BTW Andrew, while writing that I realized that there's also overlap between > your new Server classes and Trio's ABC for servers, and I think it'd be > interesting to chat about how they compare maybe? But it's not relevant to > this issue, so maybe gitter or another issue or something? If possible please use email/bpo/github. I don't use gitter and I'd like to be part of that discussion (or at least be able to follow it). ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue38242> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
