Skip to content
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

fix: Replace asyncio.gather() with aiotools.PersistentTaskGroup() for kernel creation tasks #1129

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

rapsealk
Copy link
Member

@rapsealk rapsealk commented Mar 6, 2023

This PR resolves #1128

@rapsealk rapsealk added type:bug Reports about that are not working comp:agent Related to Agent component labels Mar 6, 2023
@rapsealk rapsealk added this to the 23.03 milestone Mar 6, 2023
@rapsealk rapsealk requested a review from achimnol March 6, 2023 02:28
@rapsealk rapsealk self-assigned this Mar 6, 2023
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to refactor this part to use aiotools.PersistentTaskGroup.

@rapsealk rapsealk requested a review from achimnol March 8, 2023 07:31
@rapsealk rapsealk changed the title fix: Add BaseException to the checklist to filter out asyncio.CancelledError raised during kernel creation fix: Replace asyncio.gather() with aiotools.PersistentTaskGroup() for kernel creation tasks Mar 9, 2023
@achimnol
Copy link
Member

I have published aiotools v1.6.0 which introduces as_completed_safe() based on PersistentTaskGroup for this PR's use case.

Though, it requires Python 3.11 to handle timeouts correctly (this may be fixable but I don't have enough time now...) while its basic functionality still works with Python 3.10.

@rapsealk
Copy link
Member Author

It spreads a traceback message when an Exception has raised. It works as expected except for this.

(venv) rapsealk@MacBook-Pro aiolabs % python main.py
Traceback (most recent call last):
  File "/Users/rapsealk/Desktop/git/aiolabs/venv/lib/python3.10/site-packages/aiotools/taskgroup/persistent_compat.py", line 163, in _task_wrapper
    ret = await coro
  File "/Users/rapsealk/Desktop/git/aiolabs/main.py", line 16, in random_task
    raise Exception(str(id))
Exception: 3
INFO:__main__:results: [0, 1, 2]
INFO:__main__:errors: [Exception('3')]

@CLAassistant
Copy link

CLAassistant commented Mar 26, 2023

CLA assistant check
All committers have signed the CLA.

@Yaminyam Yaminyam added the size:M 30~100 LoC label Apr 13, 2023
@achimnol
Copy link
Member

This is on hold until I finish refactoring of aiotools with Supervisor and TaskScope.

@achimnol achimnol added the action:on hold Hold it. Wait for the restart. label Jun 30, 2023
@achimnol
Copy link
Member

Under experimentation at achimnol/aiotools#58.

@kyujin-cho kyujin-cho modified the milestones: 23.03, 23.09 Aug 30, 2023
@rapsealk rapsealk modified the milestones: 23.09, 24.03 Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:on hold Hold it. Wait for the restart. comp:agent Related to Agent component size:M 30~100 LoC type:bug Reports about that are not working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agent server fails to handle asyncio.CancelledError from create_kernels()
5 participants