Issue39529
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2020-02-02 11:11 by asvetlov, last changed 2022-04-11 14:59 by admin. This issue is now closed.
Pull Requests | |||
---|---|---|---|
URL | Status | Linked | Edit |
PR 23554 | merged | serhiy.storchaka, 2020-11-29 11:10 |
Messages (21) | |||
---|---|---|---|
msg361229 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2020-02-02 11:11 | |
Yuri proposed it for Python 3.8 but at that time the change was premature. Now we can reconsider it for 3.9 The problem is that asyncio.get_event_loop() not only returns a loop but also creates it on-demand if the thread is main and the loop doesn't exist. It leads to weird errors when get_event_loop() is called at import-time and asyncio.run() is used for asyncio code execution. get_running_loop() is a much better alternative when used *inside* a running loop, run() should be preferred for calling async code at top-level. Low-level new_event_loop()/loop.run_until_complete() are still present to run async code if top-level run() is not suitable for any reason. asyncio.run() was introduced in 3.7, deprecation on get_event_loop() in 3.8 was able to complicate support of 3.5/3.6 by third-party libraries. 3.5 now reached EOL, 3.6 is in the security-fix mode and going close to EOL. Most people are migrated to newer versions already if they care. The maintenance burden of the introduced deprecation should be pretty low. |
|||
msg361245 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2020-02-02 16:11 | |
Will asyncio.get_event_loop() be removed or made an alias of asyncio.get_running_loop()? The latter could minimize the disruption of the existing code. |
|||
msg361247 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2020-02-02 16:44 | |
Currently, I'm talking about adding a deprecation only. The asyncio.get_event_loop() function will stay in Python for a while in deprecated status. I don't know the exact period but it should be 3 releases at least I guess, with possibly extending to 5 releases if needed. |
|||
msg361248 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2020-02-02 16:49 | |
Serhiy, maybe I had not understood your proposal properly. If you are asking about deprecating get_event_loop() call from outside of async code but implicit call get_running_loop() without a warning if called from async function -- it sounds like a brilliant idea. Something like: def get_event_loop(): current_loop = _get_running_loop() if current_loop is not None: return current_loop warnings.warn("...", DeprecationWarning) return get_event_loop_policy().get_event_loop() |
|||
msg361249 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2020-02-02 17:12 | |
Yes, it is what I meant. Many code written before 3.7 use get_event_loop() because there was no get_running_loop() yet. Now, to avoid deprecation (and making the code more robust) it should be rewritten with using get_running_loop() or get_event_loop() depending on Python version. It is cumbersome, and causes a code churn. |
|||
msg361609 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2020-02-07 19:32 | |
I think we still use get_event_loop() in asyncio code, no? If we do, we should start by raising deprecation warnings in those call sites, e.g. if a Future or Lock is created outside of a coroutine and no explicit event loop is passed. We should do this in 3.9. We can then think about deprecating get_event_loop in 3.10. |
|||
msg362487 - (view) | Author: Kyle Stanley (aeros) * | Date: 2020-02-23 02:57 | |
FWIW, I agree with get_event_loop() being problematic with its error messages, and having a bit too much functionality in a single function from an API design perspective. It commonly comes up as an issue among asyncio users in communities that I'm active in. Typically, I advise them to use get_running_loop() where possible, as it can be directly substituted within a coroutine func. For __init__ methods, I recommend setting their internal loop attribute to None and then setting it to get_running_loop() in either a coro start() method or coro factory method. Outside of directly managing the event loop, this works for many use cases. When it's needed to actually create one, I advise them to use new_event_loop(), but mention that it's already handled if they're using asyncio.run(). Andrew Svetlov wrote: > The asyncio.get_event_loop() function will stay in Python for a while in deprecated status. I don't know the exact period but it should be 3 releases at least I guess, with possibly extending to 5 releases if needed. With how many libraries that rely on it, I suspect it will likely be a very slow transition from deprecation to removal. 4 versions seems like a reasonable period to me, but I think that 3 may be too short (assuming we retain the newer annual release cycle). Yury Selivanov wrote: > I think we still use get_event_loop() in asyncio code, no? Indeed, we currently use it in several places throughout asyncio. From a brief glace using git grep "get_event_loop()": Lib/asyncio/futures.py:76: self._loop = events.get_event_loop() Lib/asyncio/futures.py:390: loop = events.get_event_loop() Lib/asyncio/locks.py:81: self._loop = events.get_event_loop() Lib/asyncio/locks.py:177: self._loop = events.get_event_loop() Lib/asyncio/locks.py:244: self._loop = events.get_event_loop() Lib/asyncio/locks.py:375: self._loop = events.get_event_loop() Lib/asyncio/queues.py:35: self._loop = events.get_event_loop() Lib/asyncio/streams.py:45: loop = events.get_event_loop() Lib/asyncio/streams.py:82: loop = events.get_event_loop() Lib/asyncio/streams.py:104: loop = events.get_event_loop() Lib/asyncio/streams.py:120: loop = events.get_event_loop() Lib/asyncio/streams.py:147: self._loop = events.get_event_loop() Lib/asyncio/streams.py:403: self._loop = events.get_event_loop() Lib/asyncio/subprocess.py:206: loop = events.get_event_loop() Lib/asyncio/subprocess.py:227: loop = events.get_event_loop() Lib/asyncio/tasks.py:69: loop = events.get_event_loop() Lib/asyncio/tasks.py:129: loop = events.get_event_loop() Lib/asyncio/tasks.py:590: loop = events.get_event_loop() Lib/asyncio/tasks.py:669: loop = events.get_event_loop() Lib/asyncio/tasks.py:751: loop = events.get_event_loop() For brevity, I omitted the docs, tests, and the function definition for get_event_loop(). Based on Serhiy's idea (of making get_event_loop() an alias for get_running_loop() without warning inside of a coro func, but warning for using it outside of one), many of the above could remain as is to reduce some code churn. We just have to make sure the documentation is updated to reflect get_event_loop() becoming an alias for get_running_loop(), at the same time as the deprecation warning is added for using it outside of a coro func. Otherwise, I suspect it could lead to significant confusion from users that have warnings enabled. That being said, I think we should eventually remove asyncio.get_event_loop() entirely from the asyncio internals, including the ones that wouldn't raise deprecation warnings (if/when it's made an alias to get_running_loop()) for improved clarity. Personally, I find that even the name get_event_loop() is rather vague; get_running_loop() is much more obvious as to its purpose and what it does from a readability perspective. Yury Selivanov wrote: > If we do, we should start by raising deprecation warnings in those call sites, e.g. if a Future or Lock is created outside of a coroutine and no explicit event loop is passed. For asyncio.Lock (plus other synchronization primitives) and asyncio.Queue, this would be added in https://github.com/python/cpython/pull/18195. Currently waiting on emanu (author of the PR) to finish up some changes, but it's mostly complete. Could I work on adding it to asyncio.Future and other classes in the meantime? One point to be clarified though: you mention "created outside of a coroutine and no explicit event loop is passed". However, there are already several deprecations in place for passing an explicit event loop for most (if not all) of the __init__ methods for objects across asyncio's high-level API. In those cases, should the deprecation for creating the object outside of a coroutine function care about whether or not an explicit event loop is passed? I can see why it would matter for the lower level parts of the API (such as asyncio.Future) where passing the event loop explicitly is still allowed, but IMO it shouldn't be a factor for ones where passing the event loop explicitly is already deprecated. Especially considering that the loop argument will be removed from those entirely in 3.10 (according to the version listed in the current deprecation warnings added in 3.8). |
|||
msg362674 - (view) | Author: Yury Selivanov (yselivanov) * | Date: 2020-02-26 00:04 | |
> For asyncio.Lock (plus other synchronization primitives) and asyncio.Queue, this would be added in https://github.com/python/cpython/pull/18195. Currently waiting on emanu (author of the PR) to finish up some changes, but it's mostly complete. Could I work on adding it to asyncio.Future and other classes in the meantime? I think the approach should be different: # leading underscore is significant: loop = asyncio._get_running_loop() if loop is None: issue_deprecation_warning() loop = asyncio.get_event_loop() |
|||
msg381860 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2020-11-25 20:21 | |
I am writing a code (it is not trivial to emit correct warning) and I have a question. What to do if the loop was set by set_event_loop()? Currently get_event_loop() can return a loop without creating a new loop while get_running_loop() raises an error. Should get_event_loop() still support this in future or set_event_loop() will be deprecated? |
|||
msg381881 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2020-11-26 07:31 | |
I think the deprecation of `set_event_loop()` is a good idea. The function is not required by `asyncio.run()` implementation. |
|||
msg382044 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2020-11-29 08:03 | |
The most visible effect in tests is that asyncio.gatcher() and some other synchronous functions will need a running loop if any of arguments is a coroutine. |
|||
msg391851 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2021-04-25 10:40 | |
New changeset 172c0f2752d8708b6dda7b42e6c5a3519420a4e8 by Serhiy Storchaka in branch 'master': bpo-39529: Deprecate creating new event loop in asyncio.get_event_loop() (GH-23554) https://github.com/python/cpython/commit/172c0f2752d8708b6dda7b42e6c5a3519420a4e8 |
|||
msg394732 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2021-05-29 17:45 | |
> The maintenance burden of the introduced deprecation should be pretty low. This is going to cause an unpleasant amount of churn in the Tornado community. It's been idiomatic (going back 12 years now) to do all your setup in synchronous code before starting the event loop. I'm tempted to restore the implicit loop creation in Tornado's `IOLoop.current()` (which is now essentially a wrapper around `asyncio.get_event_loop()`). > It leads to weird errors when get_event_loop() is called at import-time and asyncio.run() is used for asyncio code execution. *Checks asyncio.run's implementation* Oh, I can see how that's a problem (and I've been giving people bad advice about the interchangeability of Tornado's IOLoop.run_sync and asyncio.run). But why does `asyncio.run` unconditionally create a new event loop instead of running on `asyncio.get_event_loop`? If asyncio.run used asyncio.get_event_loop it seems to me that we'd have the expected behavior and wouldn't have to make breaking changes to get_event_loop. > Low-level new_event_loop()/loop.run_until_complete() are still present to run async code if top-level run() is not suitable for any reason. What if you're using run_forever instead of run_until_complete? (this is the common mode of operation for Tornado) There are solutions, of course (my preferred one is `await asyncio.Event().wait()`), but it's an extra conceptual hurdle to throw at users in a "hello world" example and this is why I've stuck with the model that uses (the equivalent of) run_forever instead of asyncio.run. |
|||
msg394821 - (view) | Author: Kyle Stanley (aeros) * | Date: 2021-05-31 18:16 | |
> But why does `asyncio.run` unconditionally create a new event loop instead of running on `asyncio.get_event_loop`? AFAIK, it does so for purposes of compatibility in programs that need multiple separate event loops and providing a degree of self-dependency. asyncio.run() is entirely self-reliant in that it creates all needed resources at the start and closes them in finalization, rather than depending on existing resources. I believe this to be significantly safer and better guaranteed to function as intended, although perhaps at some cost to convenience in cases like your own where there only needs to be one event loop. |
|||
msg407429 - (view) | Author: Min RK (minrk) * | Date: 2021-12-01 07:55 | |
The comments in this thread suggest that `set_event_loop` should also be deprecated, but it hasn't been. It doesn't seem to have any use without `get_event_loop()`. I'm trying to understand the consequences of these changes for IPython, and make the changes intended by asyncio folks, but am not quite clear, yet. If I understand it correctly, this means that the whole concept of a 'current' event loop is deprecated while no event loop is running? My interpretation of these changes is that it means any persistent handles on any event loop while it isn't running is fully the responsibility of individual libraries (e.g. tornado, IPython). This is coming up in IPython where we need a handle on the event loop and advance it with `run_until_complete` for each iteration (it should be the same loop to maintain persistent state across advances, so `asyncio.run()` would not be appropriate). We previously relied on `get_event_loop` to manage this handle, but I think we have to now shift to tracking our own handle, and can no longer rely on standard APIs to track a shared instance across packages. |
|||
msg407430 - (view) | Author: Min RK (minrk) * | Date: 2021-12-01 08:22 | |
Further digging reveals that `policy.get_event_loop()` is _not_ deprecated while `asyncio.get_event_loop()` is. Is that intentional? Does that mean switching our calls to `get_event_loop_policy().get_event_loop()` should continue to work without deprecation? |
|||
msg407432 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2021-12-01 09:05 | |
IMHO, asyncio.set_event_loop() and policy.get_event_loop()/policy.set_event_loop() are not deprecated by oversight. In IPython, I think you could use new_event_loop() for getting a new loop instance. Then, save the loop reference somewhere as a direct attribute, threading.local or ContextVar. Calling loop.run_until_complete() looks pretty normal in your situation. At my job, we have Runner class, the basic usage is: with Runner() as runner: runner.run(async_func()) The implementation is pretty close to asyncio.run() but runner.run(...) can be called multiple times. Async context manager interface is responsible for resource closing. Maybe I should extract the implementation into a third-party library, I've found this concept useful for CLI applications at least. |
|||
msg407441 - (view) | Author: Min RK (minrk) * | Date: 2021-12-01 11:19 | |
Thank you! I think I have enough information to update. > IMHO, asyncio.set_event_loop()...[is] not deprecated by oversight. I'm curious, what is an appropriate use of `asyncio.set_event_loop()` if you can never get the event loop with `get_event_loop()`? If you always have to pass the handle around anyway, I'm not sure what the use case for a write-only global would be. |
|||
msg407448 - (view) | Author: Andrew Svetlov (asvetlov) * | Date: 2021-12-01 12:23 | |
Ages ago, get_event_loop() did not return the current running loop if called from async function context; it returned a loop instance registered with previous set_event_loop(...) call instead. Later we fixed get_event_loop() behavior in 3.5.4 IIRC and added get_running_loop() in 3.7 set_event_loop() doesn't make sense now from my perspective. |
|||
msg407453 - (view) | Author: Min RK (minrk) * | Date: 2021-12-01 12:40 | |
Oops, I interpreted "not deprecated by oversight" as the opposite of what you meant. Sorry! All clear, now. |
|||
msg407600 - (view) | Author: Ben Darnell (Ben.Darnell) * | Date: 2021-12-03 19:52 | |
> In IPython, I think you could use new_event_loop() for getting a new loop instance. > Then, save the loop reference somewhere as a direct attribute, threading.local or ContextVar. > Calling loop.run_until_complete() looks pretty normal in your situation. That works if you're a leaf in the dependency tree, but what about a library like Tornado (which IPython depends on)? If intermediate libraries (say Tornado and Twisted) introduce their own thread-local loops for backwards compatibility of their existing interfaces, the ecosystem gets fragmented again. Having the thread-local live in asyncio is useful for seamless interoperability across frameworks. > asyncio.run() is entirely self-reliant in that it creates all needed resources at the start and closes them in finalization, rather than depending on existing resources. I believe this to be significantly safer and better guaranteed to function as intended, although perhaps at some cost to convenience in cases like your own where there only needs to be one event loop. Whether or not this is more likely to function as intended depends on assumptions about user expectations. In Tornado (and other async systems I have used in the past), it is the norm to set up handlers on an event loop while it is not running and then start it afterwards. The behavior of asyncio.run is surprising to me. Maybe I'm weird, but regardless of which behavior is surprising to the fewest people, my point is that this change is very disruptive to Tornado and most applications using it, contrary to the claim that the maintenance burden should be pretty low. I realize it may be too late to change course, but my preference would have been to resolve the conflict between get_event_loop and asyncio.run by making asyncio.run essentially an alias for asyncio.get_event_loop().run_until_complete. This would relax the isolation of asyncio.run, but that doesn't seem very important to me. The comparable idioms in Tornado (using the IOLoop.run_sync method) all work this way and I've never seen any confusion related to this or a situation in which creating a brand-new event loop in run_sync would be desirable. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:59:26 | admin | set | github: 83710 |
2022-02-25 11:20:12 | johnmellor | set | nosy:
+ johnmellor |
2021-12-03 19:52:45 | Ben.Darnell | set | messages: + msg407600 |
2021-12-01 12:40:43 | minrk | set | messages: + msg407453 |
2021-12-01 12:23:40 | asvetlov | set | messages: + msg407448 |
2021-12-01 11:19:52 | minrk | set | messages: + msg407441 |
2021-12-01 09:05:49 | asvetlov | set | messages: + msg407432 |
2021-12-01 08:22:53 | minrk | set | messages: + msg407430 |
2021-12-01 07:55:52 | minrk | set | nosy:
+ minrk messages: + msg407429 |
2021-05-31 18:16:28 | aeros | set | messages: + msg394821 |
2021-05-29 17:45:20 | Ben.Darnell | set | nosy:
+ Ben.Darnell messages: + msg394732 |
2021-04-25 10:43:30 | serhiy.storchaka | set | status: open -> closed type: enhancement stage: patch review -> resolved resolution: fixed versions: + Python 3.10 |
2021-04-25 10:40:51 | serhiy.storchaka | set | messages: + msg391851 |
2020-11-29 11:10:47 | serhiy.storchaka | set | keywords:
+ patch stage: patch review pull_requests: + pull_request22435 |
2020-11-29 08:03:23 | serhiy.storchaka | set | messages: + msg382044 |
2020-11-26 07:31:25 | asvetlov | set | messages: + msg381881 |
2020-11-25 20:21:49 | serhiy.storchaka | set | messages: + msg381860 |
2020-02-26 00:04:02 | yselivanov | set | messages: + msg362674 |
2020-02-23 02:57:28 | aeros | set | nosy:
+ aeros messages: + msg362487 |
2020-02-07 19:32:10 | yselivanov | set | messages: + msg361609 |
2020-02-07 19:05:39 | xtreak | set | nosy:
+ xtreak |
2020-02-02 17:12:56 | serhiy.storchaka | set | messages: + msg361249 |
2020-02-02 16:50:06 | asvetlov | set | nosy:
+ yselivanov components: + asyncio |
2020-02-02 16:49:58 | asvetlov | set | messages: + msg361248 |
2020-02-02 16:44:08 | asvetlov | set | messages: + msg361247 |
2020-02-02 16:11:17 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg361245 |
2020-02-02 11:11:37 | asvetlov | create |