-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
DeprecationWarning scope expanded in asyncio.events #100160
Comments
@kumaraditya303 @serhiy-storchaka Looks like the 3.12 code somehow made it into 3.11.1 (and 3.10.9). We should roll this back. the culprit appears to be #93453. |
I am confused, yes there is a change in behavior but it is correct. You need to set the event loop before getting it alternatively use the runner APIs. I looked at the code and it is missing setting the event loop: |
In pre-3.12 semantics, the first ever |
We shouldn't have a change in behavior between 3.11.0 and 3.11.1. |
It was an intentional change based on results of discussion in #93453. A user is expected to only use |
I'm guessing it's unfortunate that we have a deprecation warning in 3.11.1 that wasn't there in 3.11.0, and ditto for 3.10.9 vs. 3.10.8. That's not normally how we do deprecations (unless it's security related). I'd like an opinion from an SC member -- let me tag @gpshead, he can decide whether to escalate this to the SC agenda or whatever. |
New warnings from existing code paths should not normally happen in patch releases. It is unfortunate that this change already shipped in 3.10.9 and 3.11.1. I recommend undoing it in both branches. Release manager @pablogsal, Is this worth rolling 3.10.10 and 3.11.2 releases for? At least two projects disrupted by the warning change have linked to this issue so far (python-prompt-toolkit and tqdm). There will presumably always be people using 3.10.9 and 3.11.1 so impacted projects may need to carry workarounds forever regardless. Questions to consider:
From #93453:
The thing to do in this kind of situation is to either leave the existing warning as is in the stable releases, or remove it entirely. (It is fine if we marked something as deprecated in a release and later change our mind and opt not to deprecate it). What I think happened here appears to be that an attempt to narrow the warning to reflect a new plan was made. This wound up moving the warning to happen from different API calls. The original released warning was removed (fine), but a new location raising the warning was added to a different call path. As seen in the tests:
Strictly speaking this was a bug that the warning wasn't raised in those releases. The lack of warning about an upcoming behavior change should be considered a reason to delay making the change. ie: restore the implicit event loop creation in 3.12 and remove it in 3.14. I reopened #93453 with such a comment. |
Plan A:
Plan B:
Plan C:
|
For prompt toolkit, it does affect transitive users including those of xonsh, who see the warning emitted when the shell starts. |
I think it has to be plan A. |
FWIW you can add several jupyter packages to affected users. We would also prefer plan A. |
@serhiy-storchaka Is there a PR yet? Should I label this as release blocker for the next 3.10 and 3.11 releases? |
I concur with this
I went ahead and add it myself 👍 |
Partially revert changes made in pythonGH-93453. asyncio.DefaultEventLoopPolicy.get_event_loop() now emits a DeprecationWarning and creates and sets a new event loop instead of raising a RuntimeError if there is no current event loop set.
…_loop() They are deferred to Python 3.12.
…t_loop() (#100412) Some deprecation warnings will reappear (in a slightly different form) in 3.12. Co-authored-by: Guido van Rossum <[email protected]>
So after deprecation warnings have been added and removed (and moved from one version to another), is this an accurate summary of the long-term plans? There are three non-deprecated ways to run an event loop:
The fourth, deprecated way is to use Historically,
In Python 3.10 the plan was to deprecate the second and third functions of Have I got this all right? In particular I want to confirm that get/set_event_loop is once again something that I can rely on for the long term and the changes from 3.10 are not just delayed for a few more releases. |
Yes, this is what we're planning to do. There's one thing I'm not 100% sure of, and that is that |
Great, thanks. My opinion is that if there's a running event loop that's always what you want. If you change I'd be supportive of adding warnings or errors to prevent situations in which the stashed-away event loop differs from the running one. Is it worth it? I think it probably is. One of the reasons these deprecation changes started in Python 3.10 was that it was easy to refer to the stashed-away event loop while setting up your application, and then have those references become invalid when you later call I think it's a good idea for |
I'd be wary of too many such checks, as this might be performance sensitive. But I like the idea of at least checking in @kumaraditya303 What do you think? |
After testing the latest 3.12 alpha, I've revised my opinions here. There's no good way today to get the behavior that will be the standard in 3.14. The stashed-away event loop is accessible only via So while I hate to suggest adding new interfaces in an area where we're actively trying to trim down and simplify things, if there were a function like |
Hm, it's either that or not call it from the main loop. :-) A I'd rather not add a corresponding top-level function, to minimize API bloat. |
Good news: I think the Tornado code in question is obsolete. Looking back at the history this logic appears to have been an attempt to emulate I no longer have an immediate need for a |
So the 3.12+ best api to get a the stashed loop or a new one is the following? import asyncio
import sys
import warnings
def get_event_loop() -> asyncio.AbstractEventLoop:
try:
return asyncio.get_running_loop()
except RuntimeError:
pass
if sys.version_info < (3,12):
return asyncio.get_event_loop()
else:
with warnings.catch_warnings(action='error'):
loop = None
try:
loop = asyncio.get_event_loop()
except (DeprecationWarning, RuntimeError):
pass
if loop is None:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
return loop It may be me, but would have hoped for something simpler |
That's essentially the approach we had to take in |
The problem is that Rather than the code posted by @CaselIT or the roughly equivalent Jupyter code linked by @blink1073, it's easy to write your own, dropping the "maybe create a loop" functionality. Untested: class LoopHelper(threading.local):
loop = None
helper = LoopHelper()
def get_event_loop_or_none():
try:
return asyncio.get_running_loop()
except RuntimeError:
return helper.loop
def get_event_loop_or_error():
loop = get_event_loop_or_none()
if loop is None:
raise RuntimeError(...)
return loop
def get_event_loop_or_create():
loop = get_event_loop_or_none()
if loop is None:
loop = helper.loop = asyncio.new_event_loop()
return loop (Read "get_event_loop_or_XXX" as "get the running event loop, or the thread-local stored loop if not None, or XXX".) Manipulate |
Thanks for the reply
I agree, but maybe some lower level api could make sense? Even just a "get_stashed_loop" would greatly simplify this use case. def get_event_loop():
loop = asyncio.get_stashed_loop() # or raise runtime error if missing
if loop is None:
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
return loop In the end I've managed to refactor the code to make use of Runner (but it requires implementing it for python<3.11. Not that adding a new api would be any better in this regard) |
@gvanrossum what we're trying to guard against is an |
@blink1073 If you're serious about proposing a new lower-level API, can you start a discussion on discuss.python.org? I think it's a bit out of scope for this particular issue. (Maybe wait until the new year, many people are relaxing for the coming week or so.) |
@gvanrossum I've found a better path forward - ensuring that we call |
As discovered in prompt-toolkit/python-prompt-toolkit#1696, it appears that the DeprecationWarning introduced in Python 3.10 has expanded its scope, now with 3.11.1 and 3.10.9 emitting during get_event_loop_policy() where it did not before:
It's not obvious if it was intentional to expand the scope of the DeprecationWarning, but it does come as a surprise as the calling code had previously attempted to address the deprecation.
I think there are two concerns to be addressed:
Linked PRs
The text was updated successfully, but these errors were encountered: