Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
2f4d496 to
a7bbd6c
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the SDK’s WebSocket message handling toward a more event-driven model by routing responses to per-request asyncio.Futures, aiming to improve high-concurrency stability and reduce polling/listener overhead.
Changes:
- Introduces
_pending_operationswith anOperationStatestate machine to route inbound messages to waitingFutures. - Adds a global request semaphore (
MAX_CONCURRENT_REQUESTS) and reconnect jitter/backoff adjustments. - Tweaks connection stability settings (e.g., increased ping/pong timeout) and adds disconnect-time cancellation of pending operations.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| runware/utils.py | Increases ping timeout; adds MAX_CONCURRENT_REQUESTS env-configured constant. |
| runware/types.py | Adds OperationState and request wrapper dataclasses used by retry/routing logic. |
| runware/server.py | Routes messages to pending-operation futures and cancels pending operations on close. |
| runware/base.py | Implements pending-operation registry, retry/reconnect behavior updates, and semaphore-based concurrency limiting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…callers get the result and the server response is not dropped.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sirsho1997
left a comment
There was a problem hiding this comment.
LGTM
I made some additional changes. Please review these
Problem: SDK fails with 50+ concurrent requests due to O(N) listener dispatch, polling loops starving event loop, and false disconnects from missed heartbeat pongs.
Solution:
asyncio.Futurefor O(1) message routing via_pending_operationsHashMapRUNWARE_MAX_CONCURRENT_REQUESTS)PING_TIMEOUT_DURATIONfrom 10s to 30s for stabilityResult: Stable operation at high concurrency, no more false "Connection lost" errors.