Refs #36315 -- Replaced calls to asyncio.gather with TaskGroup. #19370
Conversation
8e9df78 to
014268a
Compare
014268a to
ca58d03
Compare
ca58d03 to
682c753
Compare
682c753 to
c0f12c8
Compare
There was a problem hiding this comment.
Hi @graingert, I left one suggestion.
I noticed that the asyncio.create_task() calls in handles/asgi.py can benefit from this change as well (there are probably more places too).
| async def _gather(*coros): | ||
| if len(coros) == 0: | ||
| return [] | ||
|
|
||
| if len(coros) == 1: | ||
| return [await coros[0]] | ||
|
|
||
| async def run(i, coro): | ||
| results[i] = await coro | ||
|
|
||
| try: | ||
| async with asyncio.TaskGroup() as tg: | ||
| results = [None] * len(coros) | ||
| for i, coro in enumerate(coros): | ||
| tg.create_task(run(i, coro)) | ||
| return results | ||
| except BaseExceptionGroup as exception_group: | ||
| if len(exception_group.exceptions) == 1: | ||
| raise exception_group.exceptions[0] | ||
| raise |
There was a problem hiding this comment.
Can this be simplified to:
| async def _gather(*coros): | |
| if len(coros) == 0: | |
| return [] | |
| if len(coros) == 1: | |
| return [await coros[0]] | |
| async def run(i, coro): | |
| results[i] = await coro | |
| try: | |
| async with asyncio.TaskGroup() as tg: | |
| results = [None] * len(coros) | |
| for i, coro in enumerate(coros): | |
| tg.create_task(run(i, coro)) | |
| return results | |
| except BaseExceptionGroup as exception_group: | |
| if len(exception_group.exceptions) == 1: | |
| raise exception_group.exceptions[0] | |
| raise | |
| async def gather_tasks(*coros): | |
| async with asyncio.TaskGroup() as tg: | |
| tasks = [tg.create_task(coro) for coro in coros] | |
| return [task.result() for task in tasks] |
unless there's reason for the error handling? (BaseExceptionGroup).
Also, it might be worth moving the function to a utils.py file, so that it can easily be reused by other places.
There was a problem hiding this comment.
Yes the error handling is important to maintain backwards compatibility with the old use of gather
There was a problem hiding this comment.
Keeping an extra reference to tasks here will result in reference cycles in the traceback of any errors that will delay garbage collection if there's any errors in signals
There was a problem hiding this comment.
To maintain backward compatibility, shouldn't the first exception be raised even when the exception group contains multiple exceptions?
I have a PR for that here #19366 |
|
Don't know if this may raise some compatibility issues. |
Trac ticket number
ticket-36315
Branch description
Provide a concise overview of the issue or rationale behind the proposed changes.
Checklist
mainbranch.