Author njs
Recipients asvetlov, gvanrossum, lukasz.langa, njs, rhettinger, xtreak, yselivanov
Date 2019-09-24.00:04:06
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1569283448.09.0.5707304283.issue38242@roundup.psfhosted.org>
In-reply-to
Content
I saw Yury on Saturday, and we spent some time working through the implications of different options here.

For context: the stream ABCs will be a bit different from most third-party code, because their value is proportional to how many projects adopt them. So some kind of central standardization is especially valuable. And, they'll have lots of implementors + lots of users, so they'll be hard to change regardless of whether we standardize them (e.g. even adding new features will be a breaking change). Part of the reason we've been aggressively iterating on them in Trio is because I know that eventually we need to end up with a minimal core that can never change again, so I want to make sure we find the right one :-).

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, maybe in the stdlib or maybe just as an informational PEP that we can use to convince people that this is "the python way" (like the WSGI PEP).

Now, the implications for the asyncio.Stream API in 3.8. This is tricky because we aren't sure of how everything will shake out, so we considered multiple scenarios.

First decision point: will asyncio.Stream implement the ABCs directly, or will you need some kind of adapter? If we go with an adapter, then there's no conflict between the ABC approach and whatever we do in 3.8, because the adapter can always fix things up later. But, it might be nicer to eventually have asyncio.Stream implement the ABC directly, so users can use the recommended API directly without extra fuss, so let's consider that too.

Next decision point: will the byte stream ABC have an __aiter__ that yields chunks? We're pretty sure this is the only place where the ABC *might* conflict with the asyncio.Stream interface. And as Josh Oreman pointed out in the Trio issue thread, we could even avoid this by making the ABC's chunk-iterator be a method like .chunks() instead of naming it __aiter__.

So even with the current asyncio.Stream, there are two potentially workable options. But, they do involve *some* compromises, so what would happen if we wanted asyncio.Stream to implement the ABC directly without and adapter, *and* the ABC uses __aiter__? We can't do that with the current asyncio.Stream. Are there any tweaks we'd want to make to 3.8 to keep our options open here?

The obvious change would be to leave out __aiter__ from asyncio.Stream in 3.8. That would leave all our options open. For sockets this would be easy, because the old functions are still there and still returning StreamReader/StreamWriter objects. For 3.8, we're adding a bunch of new Stream-based APIs, and users won't encounter a Stream until they switch to those. (The new APIs are: connect, connect_read_pipe, connect_write_pipe, connect_unix, StreamServer, UnixStreamServer). The problem is the subprocess functions (create_subprocess_exec, create_subprocess_shell), since they've been changed *in place* to return asyncio.Stream instead of StreamReader/StreamWriter.

We considered the possibility of migrating the existing subprocess functions to a different __aiter__ implementation via a deprecation period, but concluded that this wasn't really workable. So if we want to go down this branch of the decision tree, then 3.8 would have to leave create_subprocess_{exec,shell} as using StreamReader/StreamWriter, and in either 3.8 or 3.9 we'd have to add new subprocess functions that use Stream, like we did for sockets.

Doing this for subprocesses is a bit more complicated than for sockets, because subprocesses have a Process object that holds the stream objects and interacts with them. But looking at the code, I don't see any real blockers.

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.

So I guess this is the real question we have to answer now. Which of these do we pick?

Option 1: Keep the Stream code as-is, and accept that using asyncio.Stream with future ABC-based code will require some compromises

Option 2: Create new functions for spawning subprocesses; revert create_subprocess_{exec,shell} to use StreamReader/StreamWriter; take advantage of the break between old and new APIs to clean up Stream in general; take advantage of that cleanup to remove __aiter__ so the the future ABC-based code won't have to compromise.

I think this is one of those good problems to have, where really we could live with either option :-). Still, we should pick one on purpose instead of by accident.

Yury's preference was "option 1", because he feels compromises for the ABC design aren't that bad, and adding and documenting new subprocess APIs is expensive.

I'm leaning towards "option 2", because we're already paying the cost of adding and documenting a ton of new APIs for sockets. Given that, adding a few more to that list doesn't add that much cost, and it lets us get more value from *all* of the new APIs, by letting us shed more of the legacy backcompat constraints. Right now we don't have any path to fixing 'write'/'close' at all; this would give us one. I'd also be willing to put in some time to actually implement this if that turns out to be the limiting factor.
History
Date User Action Args
2019-09-24 00:04:08njssetrecipients: + njs, gvanrossum, rhettinger, asvetlov, lukasz.langa, yselivanov, xtreak
2019-09-24 00:04:08njssetmessageid: <1569283448.09.0.5707304283.issue38242@roundup.psfhosted.org>
2019-09-24 00:04:08njslinkissue38242 messages
2019-09-24 00:04:06njscreate