-
Notifications
You must be signed in to change notification settings - Fork 6
refactor: decouple state setter from async callback execution #506
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
📝 WalkthroughWalkthroughRefactors WebSocket state management: replaces an async state setter with a synchronous setter that schedules async callbacks via asyncio.create_task and falls back to loop.call_soon_threadsafe. Adds an internal async _set_state helper and tests for the threadsafe fallback and import cleanup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (4.0.4)tests/test_websocket.pyThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/test_websocket.py`:
- Around line 285-306: The test function test_state_setter_threadsafe_fallback
has an unused fixture parameter mock_callback; remove mock_callback from the
test signature so it becomes async def
test_state_setter_threadsafe_fallback(ws_client): to satisfy Ruff ARG001,
leaving ws_client (which already depends on the callback fixture) intact; update
any test references if necessary and run the linter to confirm the warning is
resolved.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
Bug Fixes
Tests