Enhance subscription handling in ApiClient #329
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances WebSocket subscription handling in ApiClient by introducing a configurable heartbeat interval, refactoring the subscription loop for cleaner async context management, and updating tests to align with the new behavior.
- Add
ws_heartbeatparameter toApiClient.__init__and pass it tows_connect - Refactor
subscribeto useasync withandasync forfor WebSocket handling - Update tests to handle
WSMsgType.CLOSINGand optional exception message matching
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/test_subscription.py | Include CLOSING in message sequence, adjust exception test to accept None |
| tests/conftest.py | Add from __future__ import annotations, implement async context & iteration |
| pytraccar/client.py | Introduce ws_heartbeat, refactor subscribe with async with/async for |
Comments suppressed due to low confidence (3)
tests/test_subscription.py:180
- [nitpick] Rename the parameter
raisestoexpected_exceptionfor clarity and to avoid confusion with pytest'sraisescontext manager.
async def test_subscription_exceptions(
tests/test_subscription.py:184
- [nitpick] Consider renaming
with_messagetoexpected_messageto clearly convey its role in matching the exception message.
with_message: str | None,
pytraccar/client.py:51
- Add a test to verify that the
ws_heartbeatparameter is correctly forwarded tows_connectwhen subscribing to ensure the new feature is covered.
ws_heartbeat: int = 120,
📝 WalkthroughWalkthroughThe changes introduce a configurable WebSocket heartbeat interval to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApiClient
participant WebSocket
User->>ApiClient: subscribe(callback)
ApiClient->>WebSocket: async with ws_connect(heartbeat=ws_heartbeat)
loop For each message
WebSocket-->>ApiClient: msg
ApiClient->>ApiClient: Check msg.type
alt msg.type is TEXT
ApiClient->>callback: await callback(data)
else msg.type is CLOSE/CLOSED/CLOSING/ERROR
ApiClient->>User: Raise TraccarConnectionException
end
end
sequenceDiagram
participant Test
participant MockedWSContext
Test->>MockedWSContext: async with (context manager)
MockedWSContext-->>Test: __aenter__ returns self
loop For each message
MockedWSContext-->>Test: __anext__ yields next message
end
MockedWSContext-->>Test: __aexit__ on exit
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/conftest.py (1)
55-68: Consider improving type annotations for the async methods.The async context manager and iterator implementation is correct and properly mocks the WebSocket behavior. However, the type annotations could be improved.
Apply this diff to improve type annotations:
+from typing import Self + class MockedWSContext: # ... existing code ... - async def __aenter__(self) -> MockedWSContext: + async def __aenter__(self) -> Self: return self - async def __aexit__(self, *args: Any) -> None: + async def __aexit__(self, *args: object) -> None: pass - def __aiter__(self) -> MockedWSContext: + def __aiter__(self) -> Self: return selfNote:
Selfrequires Python 3.11+ orfrom typing_extensions import Selffor earlier versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
pytraccar/client.py(3 hunks)tests/conftest.py(2 hunks)tests/test_subscription.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/test_subscription.py (4)
tests/common.py (1)
WSMessage(13-23)tests/conftest.py (1)
api_client(89-98)pytraccar/client.py (2)
subscription_status(63-65)subscribe(158-236)pytraccar/models/subscription.py (1)
SubscriptionStatus(14-20)
tests/conftest.py (1)
tests/common.py (1)
get(49-55)
pytraccar/client.py (2)
pytraccar/models/subscription.py (1)
SubscriptionStatus(14-20)pytraccar/exceptions.py (1)
TraccarConnectionException(12-13)
🪛 Ruff (0.11.9)
tests/conftest.py
55-55: __aenter__ methods in classes like MockedWSContext usually return self at runtime
Use Self as return type
(PYI034)
58-58: Star-args in __aexit__ should be annotated with object
Annotate star-args with object
(PYI036)
🪛 Pylint (3.3.7)
tests/conftest.py
[error] 55-55: Undefined variable 'MockedWSContext'
(E0602)
[error] 61-61: Undefined variable 'MockedWSContext'
(E0602)
🔇 Additional comments (5)
tests/test_subscription.py (2)
81-81: LGTM!Adding
CLOSINGto the stopping messages is consistent with the updated WebSocket handling inclient.py.
175-175: Good improvement to exception testing flexibility.The change to allow
Noneforwith_messageenables testing exceptions without requiring message matching, which is useful for cases where the exception is raised without a message.Also applies to: 184-194
tests/conftest.py (1)
3-3: LGTM!Adding
from __future__ import annotationsenables forward reference support for type annotations.pytraccar/client.py (2)
51-51: Good addition of configurable WebSocket heartbeat.The
ws_heartbeatparameter with a default of 120 seconds provides flexibility for WebSocket connection management while maintaining a sensible default.Also applies to: 60-60
165-201: Excellent refactoring to use async context management and iteration.The changes improve the WebSocket handling by:
- Using
async withfor proper resource management- Replacing manual
receive()loop withasync foriteration- Including the heartbeat parameter in the connection
- Adding
CLOSINGto the stopping message types for comprehensive connection closure handling
No description provided.