-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
asyncio: Re-reverse deprecation of set_event_loop? #130322
Comments
@kumaraditya303 I am fine with keeping these, assuming there are no major implementation hurdles or terrible flaws. |
This is a temporary measure to get CI passing while the fate of these deprecation warnings is decided in python/cpython#130322
Python 3.14 deprecates the asyncio event loop policy system, so make (most of) the necessary changes. The deprecation of set_event_loop is extremely disruptive to AsyncTestCase, so I've asked if it can remain undeprecated in python/cpython#130322. The testing.py changes are temporary until this is resolved. Fixes #3458
In previous versions
In current Python 3.14, the child watchers have been removed entirely and policy is deprecated as such FWIW, unlike |
Okay, then what does Tornado have to do? Just have its own per-thread datastructure to hold its preferred event loop? |
This is where we differ - to me, policies concern the creation of an event loop, and the concept of a current-but-not-running event loop is something separate (which was located in the event loop policy as an implementation detail). I found a better thread where I laid all this out (and at the time @gvanrossum agreed): #100160 (comment)
That works as long as the application only uses Tornado's interfaces. Hybrid apps that mix Tornado with, for instance, Twisted are pretty rare (Glyph and I care about this but I'm not sure anyone else does). The problem is that because Tornado uses asyncio interfaces internally (and they are increasingly part of our documented best practices), mixing and matching is more of a concern. Maybe this doesn't matter, though? I need to do some more testing but I don't currently have any examples of something we use that works today in this not-yet-running state but is likely to break in the future. The things that are important to us are not higher-level interfaces (which generally require an For more context, this comes up in two different places. First is at application startup time, when the Tornado convention was to start your server (adding an event handler to the listening socket) before starting the event loop. This part I'm not as worried about because even if it's an issue, it's by definition something that can be fixed once per app, and it's fairly straightforward. The trickier part is in tests. The still-predominant testing style in Tornado web apps doesn't use any coroutines in the tests, but it all runs in synchronous mode with the application server and HTTP client started before the event loop (and then we run the loop until the HTTP fetch is complete and stop it again). If this stopped working it would be very tedious to refactor all the tests that rely on it. |
Even if you treat is an implementation detail, it was always a part of policy system so it was deprecated with it.
All of those functions would continue to work as they work currently, they are unaffected by deprecation of policy system. In general, it would be better to use asyncio.Runner APIs for versions that support it as that would handle everything for you. I am closing this as this is the intended behaviour here for reasons I described here #130322 (comment) |
I believe that the module-level get_event_loop and set_event_loop could easily be kept around, using module-level thread-local state to store the "current" event loop. This is what I take the '22 decision quoted by @bdarnell proposed to do. (Note that get_event_loop would no longer create an event loop, ever.) And we apparently never deprecated set_event_loop. I disagree that it is part of the policy system -- it just used the current policy when the policy wasn't fixed. However, the plan to change get_event_loop to become an alias for get_running_loop would get in the way of such an implementation. Was this plan (to make it an alias) ever documented? I can't find it in the 3.13 or 3.14 docs for the module-level get_event_loop -- it still mentions get_event_loop_policy().get_event_loop(). The alternative would be for Tornado and other frameworks that have their own equivalent of asyncio.run() to write their own {get,set}_event_loop using thread-local storage inside Tornado, but this might not support all intended use cases (maybe Tornado's architecture allows sharing the current event loop with other frameworks?). Let's keep this issue open until we have more of an agreement on the way forward. |
That is how I understood it as well.
Yes, and I support this part of the changes.
I don't think it was ever explicitly documented because it's kind of implied by the removal of set_event_loop. Without either set_event_loop or policies, there's no reason for get_event_loop and get_running_loop to differ. (You and I) have each commented on merging get_event_loop with get_running_loop in a world without set_event_loop). If we keep set_event_loop, we obviously need some way to get that saved event loop. This means either A) get_event_loop is not an alias for get_running_loop (it should return the running loop if there is one, or the saved loop if none is running) or B) get_event_loop is an alias for get_running_loop and a separate get_saved_loop (or some other name) gives you the saved loop.
Yes, the ability to share the saved event loop across frameworks is why we'd prefer this to be We need to be able to construct asyncio objects while no event loop is running. In particular, that includes |
It seems to me that Ben and I are in complete agreement on this issue. @kumaraditya303, are you willing to reinstate get/set_event_loop with the semantics of (eventually) storing the loop in an asyncio-wide thread-local variable? We wouldn't have to do that until policies are actually ripped out. It would be a simple change, and there's time to undeprecate the parts that will remain public APIs before 3.14 beta1 rolls around. |
@gvanrossum:
That will not work, even if
I would suggest you to try to adapt tornado to use runner APIs first, I would like to see concrete code which you cannot migrate to runner API if any before proceeding with any rolling back of deprecation. |
I am not a fan of adding more thread local storage and continuing this weird state of "set loop but not running thing" even if it is thread specific in asyncio. I am -1 on that for now unless we have no other way to fix this. This can be solved in tornado itself by relying on the use of current event loop and not calling Sample code: loop_data = {}
def loop_factory():
loop = asyncio.EventLoop()
tornado_data = ...
loop_data[loop] = tornado_data
return data
asyncio.run(coro, loop_factory=tornado.loop_factory) |
Correct, but what I meant was to call
Not always - But acknowledged that if we go this route the wrapper should call
There are two problems with runner from my perspective: First is just that it's too new - I still support versions of python that don't have it. But the bigger problem is the design of Tornado's AsyncHTTPTestCase. This is ancient code that predates coroutines but was never deprecated and is still the recommended way to write tests of Tornado apps. It simulates coroutines with synchronous methods that run the event loop for you. For example:
So if we lose
This is a bit of a digression since our issue does not involve attaching information to the event loop, but I just want to emphasize the limitations of solving things "in tornado itself". Tornado is a framework; it does not call |
Feature or enhancement
Proposal:
The asyncio policy system is deprecated in python 3.14 (#127949). As implemented, this includes the
asyncio.set_event_loop()
function. However, in 2022, it was decided thatset_event_loop
andget_event_loop
(just the thread-local storage, not the broader policy system) were serving a useful and separate purpose and should be kept around. I relied on this decision in Tornado to adapt to various changes while staying on interfaces that I thought would be safe from deprecation.I would like to briefly bring this up for reconsideration since this is a reversal of a decision from just a few years ago and it appears to have been lumped in with the rest of the policy system without consideration on its own.
If the decision to deprecate this function stands, I'll be able to adapt in Tornado, but it will be inconvenient: I think I'll have to use asyncio.Runner which was introduced in 3.11, while I'm still supporting 3.9 and 3.10 for another couple of years.
Background
Here's the history as I remember it. In Python 3.10, the policy system was deprecated and slated for removal in 3.12. When the 3.12 alphas were released with the policy methods removed, we found that there were code paths that relied on the deprecated methods without emitting suitable warnings, leading to surprise breakages. This caused everything to be reset (in 3.10.9 and 3.11.1, which is why those specific versions are cited in the docs). Deprecation notices were removed, but the understanding at the time was that this was just resetting the clock and policies would still be going away, just a few years later.
Separately, there was a debate about the set_event_loop function, although I can't find the right thread now. (#83710 (comment) and #98440 (comment) are related, but I can't find where the actual conclusion was reached). I think Jupyter was another project where
set_event_loop
turned out to be important?Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
No response
The text was updated successfully, but these errors were encountered: