Skip to content

Conversation

@0oshowero0
Copy link
Collaborator

@0oshowero0 0oshowero0 commented Jan 26, 2026

Background

The existing sync APIs in TransferQueueClient are implemented by wrapping the original async methods in AsyncTransferQueueClient with asyncio.run().

This works when the caller is in a purely synchronous context. However, in environments where an event loop is already running (for example, Jupyter Notebook), calling these sync APIs will fail unless users manually switch to the async APIs.

Solution

This PR replaces the direct use of asyncio.run() by creating a dedicate event loop in a new thread.

This allows the sync APIs in TransferQueueClient to work reliably in both synchronous and asynchronous environments without requiring changes from the user.

TODO

  • Unify the client usage in upper frameworks by TransferQueueClient since it supports both sync & async APIs.

CC: @vermouth1992 @wuxibin89

Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
Copilot AI review requested due to automatic review settings January 26, 2026 06:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the synchronous TransferQueueClient API so it can safely be used from environments where an asyncio event loop is already running (e.g., Jupyter), by switching from asyncio.run() to asgiref’s async_to_sync. It also adds asgiref as an explicit dependency.

Changes:

  • Replace direct asyncio.run() calls in TransferQueueClient sync methods with instance-bound wrappers generated via asgiref.sync.async_to_sync.
  • Introduce a private _bind_sync_methods helper invoked in TransferQueueClient.__init__ to bind all sync wrappers in one place.
  • Add asgiref to requirements.txt and remove the unused asyncio import from transfer_queue/client.py.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
transfer_queue/client.py Refactors sync TransferQueueClient methods to use async_to_sync wrappers instead of asyncio.run, centralizing wrapper creation in _bind_sync_methods.
requirements.txt Adds the asgiref dependency required for the new async-to-sync bridge.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@0oshowero0 0oshowero0 changed the title [feat] Improve TransferQueueClient sync API compatibility with async contexts [WIP][feat] Improve TransferQueueClient sync API compatibility with async contexts Jan 26, 2026
Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
@0oshowero0 0oshowero0 changed the title [WIP][feat] Improve TransferQueueClient sync API compatibility with async contexts [feat] Improve TransferQueueClient sync API compatibility with async contexts Jan 26, 2026
@ji-huazhong ji-huazhong merged commit ddf1fd9 into Ascend:main Jan 26, 2026
4 checks passed
@0oshowero0 0oshowero0 deleted the fix_sync branch January 26, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants