erdewit / nest_asyncio

Patch asyncio to allow nested event loops
BSD 2-Clause "Simplified" License
693 stars 79 forks source link

Upstream contribution #51

Open davidbrochart opened 3 years ago

davidbrochart commented 3 years ago

Thanks for this library, it really saves our lives. I was thinking, since it is so useful, have you considered contributing to CPython to make this feature standard in asyncio? Or do you see issues that would prevent from doing it? Sorry if there are already discussions about it, but I couldn't find any.

erdewit commented 3 years ago

There is the original discussion thread here, where the explicit decision is made to not allow nesting. The main objection by GvR is

very hard to debug problems that appeared as very infrequent and hard to predict stack overflows

This is a valid concern and care should be taking to not to exceed the recursion limit (1000 by default). When the recursion does get too deep, the long stack trace makes it very obvious what is happening though.

davidbrochart commented 3 years ago

At the same time, I don't think his argument:

I understand this will make your code slightly less elegant in some cases, but I think in the end it is for the best if you are required to define an explicit method (declared to be a coroutine)

really applies to the issue we are faced with. It is not a matter of making some code less elegant, but we have no other choice than using nest-asyncio or running another event loop in another thread (or using something like unasync), and none of them are ideal. Also, his answer is from 2014, maybe core devs would be more inclined to reconsider the issue today?

qci-amos commented 3 years ago

Maybe some people are changing their minds about this now: https://bugs.python.org/issue22239#msg398257

maartenbreddels commented 2 years ago

I think nest asyncio breaks a fundamental assumption that in if you don't await, you cannot task switch. Now an await can hide behind a sync call, making it behave similar as multithreaded code (a task switch can occur at 'any' moment). So, I really appreciate this project, but it should be used with caution.

davidbrochart commented 2 years ago

Good point, although with nest-asyncio asyncio.run(coro) could almost be replaced with await coro, with the particularity that await can now be used even if the function is not async. It's really in that particular case (the function where asyncio.run is called) that task switching can happen with nest-asyncio, while it cannot without it. But yes it makes it hard to say by just looking if the function is async.

davidbrochart commented 2 years ago

I went ahead an opened https://github.com/python/cpython/pull/93338. I hope you don't mind @erdewit, and of course if the PR is accepted you should be the author of it.

erdewit commented 2 years ago

That's a great idea @davidbrochart. I think if the asyncio authors do want nesting then it might be easier for them to modify their own code then it is to copy the (simplified) nest_asyncio code over. Probably they'll also want a flag like asyncio.allow_nesting that needs to be explicitly set first by the user, and if not set will keep the current behavior.

davidbrochart commented 2 years ago

I think if the asyncio authors do want nesting then it might be easier for them to modify their own code then it is to copy the (simplified) nest_asyncio code over.

Yes, also the equivalent changes would need to be made in the C implementation.

Probably they'll also want a flag like asyncio.allow_nesting that needs to be explicitly set first by the user, and if not set will keep the current behavior.

I agree, although it seems difficult to handle the case where two libraries want different behaviors (one wants nesting, the other not).