-
Notifications
You must be signed in to change notification settings - Fork 5
[feat] Improve TransferQueueClient sync API compatibility with async contexts
#11
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
Conversation
Signed-off-by: 0oshowero0 <o0shower0o@outlook.com>
There was a problem hiding this 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 inTransferQueueClientsync methods with instance-bound wrappers generated viaasgiref.sync.async_to_sync. - Introduce a private
_bind_sync_methodshelper invoked inTransferQueueClient.__init__to bind all sync wrappers in one place. - Add
asgireftorequirements.txtand remove the unusedasyncioimport fromtransfer_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.
TransferQueueClient sync API compatibility with async contextsTransferQueueClient sync API compatibility with async contexts
There was a problem hiding this 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>
TransferQueueClient sync API compatibility with async contextsTransferQueueClient sync API compatibility with async contexts
Background
The existing sync APIs in
TransferQueueClientare implemented by wrapping the original async methods inAsyncTransferQueueClientwithasyncio.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
TransferQueueClientto work reliably in both synchronous and asynchronous environments without requiring changes from the user.TODO
TransferQueueClientsince it supports both sync & async APIs.CC: @vermouth1992 @wuxibin89