Replaced get_event_loop() with get_running_loop()#12425
Replaced get_event_loop() with get_running_loop()#12425agronholm wants to merge 3 commits intoaio-libs:masterfrom
get_event_loop() with get_running_loop()#12425Conversation
❌ 4 Tests Failed:
View the top 2 failed test(s) by shortest run time
View the full list of 2 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Merging this PR will not alter performance
Comparing Footnotes
|
|
We should do this. I did a few of them previously. Be aware there was pushback when we did some of the others #8555 |
|
I'm seeing test failures, but most of them seem to be about testing the test harness itself. |
|
test failure in https://github.com/aio-libs/aiohttp/actions/runs/24991218031/job/73177192738?pr=12425 are because the CI is relying on the loop being created by the legacy |
|
@bdraco do you think this can be considered backportable given that it's not an API change? I've added the labels ahead of time, for now. |
|
back port it. There may be some complaints but its past time to get this done and we can't punt on it any longer. |
|
Is the consensus then that I need to make another PR to resolve the failures seen here? |
|
I think you can just do them here. |
| # Setup uvloop policy, so that every | ||
| # asyncio.get_event_loop() will create an instance | ||
| # asyncio.get_running_loop() will create an instance | ||
| # of uvloop event loop. |
There was a problem hiding this comment.
This comment isn't exactly correct, and I think it's fairly obvious what the code does, so let's just delete this comment.
There was a problem hiding this comment.
Right – I did a fairly simple search/replace operation and missed this nuance.
Only case I see here that could cause the same problem is in NamedPipeSite, which is a rare thing for users. So, probably won't get any more complaints on this one. Though maybe there's some other way of implementing that check...? |
A site is generally created after a runner has been setup, so it's also very unlikely to be called in a situation without a running loop: |
|
I just noticed that |
|
It still has odd behaviour and is discouraged. |
What do these changes do?
This is a search & replace operation that changes all uses of the
get_event_loop()function toget_running_loop().Are there changes in behavior for the user?
No
Is it a substantial burden for the maintainers to support this?
Nope
Related issue number
Checklist
CONTRIBUTORS.txtCHANGES/foldername it
<issue_or_pr_num>.<type>.rst(e.g.588.bugfix.rst)if you don't have an issue number, change it to the pull request
number after creating the PR
.bugfix: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature: A new behavior, public APIs. That sort of stuff..deprecation: A declaration of future API removals and breakingchanges in behavior.
.breaking: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc: Notable updates to the documentation structure or buildprocess.
.packaging: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc: Changes that are hard to assign to any of the abovecategories.
Make sure to use full sentences with correct case and punctuation,
for example:
Use the past tense or the present tense a non-imperative mood,
referring to what's changed compared to the last released version
of this project.