Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

Upstream contribution #51

Open
davidbrochart opened this issue Mar 15, 2021 · 8 comments
Open

Upstream contribution #51

davidbrochart opened this issue Mar 15, 2021 · 8 comments

Comments

@davidbrochart
Copy link

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
Copy link
Owner

erdewit commented Mar 18, 2021

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
Copy link
Author

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
Copy link

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

@maartenbreddels
Copy link

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
Copy link
Author

davidbrochart commented Dec 3, 2021

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
Copy link
Author

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

@erdewit
Copy link
Owner

erdewit commented May 30, 2022

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
Copy link
Author

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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants