Repository navigation
Please restore the loop param to staggered.staggered_race #124639
Description
Activity
Hello!
Thanks for the report.asyncio.staggered_raceshould not be used directly. It is a private API that is not documented anywhere in the documentation. If you want to usestaggered_race,please install the 3rd party package async-stagger: https://github.com/twisteroidambassador/async_stagger.Reacted by Andrew SvetlovIts a bit unexpected to have deprecation warnings in it given its not intended for public use. Its also not possible to use https://github.com/twisteroidambassador/async_stagger since it only supports python 3.11+ https://github.com/twisteroidambassador/async_stagger/blob/e09eeefd5d5c123d79abb8ac7f1b5583e8ff3203/setup.py#L30
FWIW, there was no deprecation emitted. That got removed right before the PR got merged. @Eclips4 should we reopen this? I guess this is too much of a breaking change for a patch release.
@bdraco, we're going to add back the
loopparameter for 3.12. (I'm not sure what the deprecation plan is, I think it's supposed to be 3.14?)Anyways, this will allow use of
eager_task_factoryon prior aiohttp versions. However, we're officially markingstaggered_raceas a private API (and will rename it to_staggered_race). Please copy the current implementation into third parties for future use, as we're not guaranteeing any future support past 3.12 (or maybe 3.13, I think that's undecided).Reacted by Kirill Podoprigora24 remaining items
What's the purpose of the loop kwarg here? Is it to avoid an expensive call to get_running_loop? If so adding the assertion will just remove that perf fix
What's the purpose of the loop kwarg here? Is it to avoid an expensive call to get_running_loop? If so adding the assertion will just remove that perf fix
At this point the concern is historical compatibility only as get_running_loop is so much faster in newer cpython.
The new implementation doesn't seem to handle multiple winners in the same iteration of the event loop as we received aio-libs/aiohappyeyeballs#100 today
Traceback (most recent call last): File "/opt/homebrew/Caskroom/miniforge/base/envs/work/lib/python3.12/site-packages/aiohappyeyeballs/_staggered.py", line 85, in run_one_coro assert winner_index is None # noqa: S101 ^^^^^^^^^^^^^^^^^^^^ AssertionErrorI was able to write a test to simulate the problem https://github.com/aio-libs/aiohappyeyeballs/blob/e3519bbebf2069eee0aff0dfde50689c742ba97f/tests/test_staggered.py#L33
loop = asyncio.get_running_loop() winners = [] finish = loop.create_future() async def coro(idx): await finish winners.append(idx) return idx coros = [partial(coro, idx) for idx in range(4)] task = loop.create_task(staggered_race(coros, delay=0.00001)) await asyncio.sleep(0.1) loop.call_soon(finish.set_result, None) winner, index, excs = await task
I ended up writing a new implementation for
aiohappyeyeballsthat should work on 3.8-3.13+ https://github.com/aio-libs/aiohappyeyeballs/blob/main/src/aiohappyeyeballs/_staggered.py- added a commit that references this issue
on Oct 1, 2024 - added a commit that references this issue
on Oct 1, 2024 Closing as the change was reverted in all branches
I would like to concur with @bdraco that the new implementation (the one being reverted here) does not handle multiple winners.
This implementation cancels the entire task group by raising an exception when a coroutine wins. This cancellation is not immediate, in the sense that other tasks in the same group may get chances to run before being cancelled, which may result in multiple winners.
I believe (but haven't checked) that my new implementation is compatible with eager task factories: https://github.com/twisteroidambassador/async_stagger/blob/master/async_stagger/stagger.py
I thought we fixed this with a separate implementation already.
Reacted by twisteroid ambassadorI thought we fixed this with a separate implementation already.
Right, I see it now. Thanks!
Reacted by Peter Bierma
Bug report
Bug description:
The following remove the
loopparam fromstaggered.staggered_race:#124390
#124574
#124573
its wasn't scheduled for removal until 3.16this never mergedCPython versions tested on:
3.12, 3.13, CPython main branch
Operating systems tested on:
No response
Linked PRs