[client] new APIs support #27
Conversation
WalkthroughCentralizes HTTP response processing and typed error mapping; adds PUT/PATCH to sync and async HTTP clients; introduces APIError hierarchy and error_from_status; refactors Message into TextMessage/DataMessage with richer serialization; adds device/health endpoints to API clients; expands tests and CI/dev deps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Caller
participant C as APIClient / AsyncAPIClient
participant H as HttpClient / AsyncHttpClient
participant S as Server
rect rgb(230,245,255)
note over U,C: device / health / remove flows
U->>C: list_devices() / health_check() / remove_device(id)
C->>H: GET /devices or /health / DELETE /devices/{id}
H->>S: HTTP request
alt 2xx (200/201)
S-->>H: 2xx + JSON
H-->>C: parsed dict -> domain.Device[] / dict / {}
C-->>U: return value
else 204
S-->>H: 204 No Content
H-->>C: {}
C-->>U: return value
else 4xx/5xx
S-->>H: status + JSON
H-->>C: raise APIError subclass (status_code + response)
C-->>U: exception
end
end
sequenceDiagram
autonumber
participant Caller
participant HC as HttpClient / AsyncHttpClient
participant S as Server
rect rgb(245,245,255)
note over Caller,HC: PUT / PATCH request flow (new verbs)
Caller->>HC: put(url, payload, headers?) / patch(...)
alt client not initialized
HC-->>Caller: raise RuntimeError
else initialized
HC->>S: HTTP PUT/PATCH JSON body
alt 2xx
S-->>HC: 2xx + JSON
HC-->>Caller: return parsed dict
else 204
S-->>HC: 204 No Content
HC-->>Caller: return {}
else 4xx/5xx
S-->>HC: status + JSON
HC-->>Caller: raise APIError subclass via error_from_status(message,status,response)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)android_sms_gateway/ahttp.py (2)
android_sms_gateway/http.py (2)
🪛 Ruff (0.14.0)android_sms_gateway/ahttp.py103-103: Avoid specifying long messages outside the exception class (TRY003) 131-131: Avoid specifying long messages outside the exception class (TRY003) 146-146: Avoid specifying long messages outside the exception class (TRY003) 215-215: Avoid specifying long messages outside the exception class (TRY003) 241-241: Avoid specifying long messages outside the exception class (TRY003) 254-254: Avoid specifying long messages outside the exception class (TRY003) android_sms_gateway/http.py113-113: Avoid specifying long messages outside the exception class (TRY003) 127-127: Avoid specifying long messages outside the exception class (TRY003) 209-209: Avoid specifying long messages outside the exception class (TRY003) 243-243: Avoid specifying long messages outside the exception class (TRY003) 257-257: Avoid specifying long messages outside the exception class (TRY003) 🔇 Additional comments (12)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
android_sms_gateway/errors.py (1)
4-16: Consider a status→exception mapping helperTo integrate with HTTP layers, add a factory to map status codes to subclasses (and default to APIError). Keeps callers simple.
Example:
_ERROR_MAP = { } def error_from_status(message: str, status: int, response: t.Optional[dict] = None) -> APIError: return _ERROR_MAP.get(status, APIError)(message, status_code=status, response=response)android_sms_gateway/client.py (1)
95-99: Optional: rename paramhttp->http_clientfor clarityMatches AsyncAPIClient and avoids confusion with imported module.
- http: t.Optional[http.HttpClient] = None, + http_client: t.Optional[http.HttpClient] = None, ... - self.http = http + self.http = http_clientandroid_sms_gateway/domain.py (1)
21-31: Docstring nits: fix typos/types
- tex_message -> text_message
- valid_until documented as str but typed datetime; clarify expected format (ISO 8601).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android_sms_gateway/ahttp.py(3 hunks)android_sms_gateway/client.py(3 hunks)android_sms_gateway/domain.py(4 hunks)android_sms_gateway/errors.py(1 hunks)android_sms_gateway/http.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
android_sms_gateway/ahttp.py (1)
android_sms_gateway/http.py (6)
put(20-22)put(104-116)put(194-208)patch(25-27)patch(118-130)patch(210-224)
android_sms_gateway/client.py (4)
android_sms_gateway/domain.py (10)
TextMessage(118-145)DataMessage(76-114)Device(226-240)from_dict(102-114)from_dict(134-145)from_dict(155-160)from_dict(172-182)from_dict(197-210)from_dict(235-240)from_dict(253-258)android_sms_gateway/encryption.py (2)
encrypt(12-13)encrypt(31-42)android_sms_gateway/ahttp.py (6)
get(7-9)get(73-81)get(166-174)delete(27-39)delete(131-138)delete(218-225)android_sms_gateway/http.py (6)
get(10-12)get(74-80)get(162-168)delete(30-40)delete(96-102)delete(186-192)
android_sms_gateway/domain.py (1)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)
android_sms_gateway/http.py (1)
android_sms_gateway/ahttp.py (6)
put(17-19)put(99-113)put(190-202)patch(22-24)patch(115-129)patch(204-216)
🪛 GitHub Actions: Python CI
android_sms_gateway/domain.py
[error] 4-4: flake8: F401 'io.BytesIO' imported but unused
android_sms_gateway/http.py
[error] 3-3: flake8: F401 'json' imported but unused
[error] 5-5: flake8: F401 '.client' imported but unused
[error] 5-5: flake8: F401 '.domain' imported but unused
[error] 144-144: flake8: F811 redefinition of unused 'client' from line 5
🪛 Ruff (0.13.3)
android_sms_gateway/ahttp.py
107-107: Avoid specifying long messages outside the exception class
(TRY003)
123-123: Avoid specifying long messages outside the exception class
(TRY003)
198-198: Avoid specifying long messages outside the exception class
(TRY003)
212-212: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/client.py
197-197: Avoid specifying long messages outside the exception class
(TRY003)
209-209: Avoid specifying long messages outside the exception class
(TRY003)
216-216: Avoid specifying long messages outside the exception class
(TRY003)
333-333: Avoid specifying long messages outside the exception class
(TRY003)
345-345: Avoid specifying long messages outside the exception class
(TRY003)
352-352: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/domain.py
55-55: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/http.py
112-112: Avoid specifying long messages outside the exception class
(TRY003)
126-126: Avoid specifying long messages outside the exception class
(TRY003)
202-202: Avoid specifying long messages outside the exception class
(TRY003)
218-218: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (11)
android_sms_gateway/ahttp.py (3)
16-25: Protocol PUT/PATCH additions look goodSignatures are consistent with post and sync client. No issues spotted.
99-114: Aiohttp PUT/PATCH mirror GET/POST correctlySession checks, raise_for_status, and JSON parsing are consistent. All good.
Also applies to: 115-130
190-216: Httpx async PUT/PATCH are correctClient checks, await, raise_for_status, and .json() usage are appropriate for httpx.AsyncClient.
android_sms_gateway/http.py (3)
19-28: Protocol PUT/PATCH additions LGTMMatches existing style and sync with async counterpart.
104-131: Requests PUT/PATCH implementations LGTMSession guard + reuse of _process_response is consistent and correct.
194-225: httpx PUT/PATCH implementations LGTMClient guard + raise_for_status + .json() chaining looks correct.
android_sms_gateway/client.py (5)
45-59: Verify DataMessage encryption semanticsYou encrypt data_message.data directly. Server must expect the encrypted token (not base64). If the API expects base64 of raw bytes, encryption may need to occur before base64-encoding, or a different field.
Do we have API docs confirming encrypted payload format for data messages?
194-205: list_devices implementation LGTMType conversion via Device.from_dict and headers are correct.
206-212: remove_device implementation LGTMDELETE with proper endpoint and headers. Good.
213-219: health_check implementation LGTMSimple GET passthrough; fine.
330-354: Async device + health endpoints LGTMAwait pattern mirrors sync implementation correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
android_sms_gateway/domain.py (2)
1-4: The past review comment regarding the need to importenumfor proper serialization inasdict(lines 63-71) remains valid. TheMessagePriorityenum needs to be serialized to its.valueto avoid JSON serialization errors.
63-71: The past review comment regarding the need to serialize enums to.valueand datetimes to.isoformat()in theasdictmethod remains critical. Without this fix, JSON serialization will fail whenpriorityorvalid_untilis set.
🧹 Nitpick comments (1)
android_sms_gateway/domain.py (1)
33-35: Consider requiring at least one message type.Both
text_messageanddata_messageare optional, allowing aMessageinstance to be created with no content. While thecontentproperty (lines 48-54) raises a runtime error when accessed, this validation could be moved to construction time for earlier failure detection.Consider adding
__post_init__validation:def __post_init__(self): if self.text_message is None and self.data_message is None: raise ValueError("Message must have either text_message or data_message")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android_sms_gateway/domain.py(4 hunks)android_sms_gateway/http.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
android_sms_gateway/http.py (1)
android_sms_gateway/ahttp.py (6)
put(17-19)put(99-113)put(190-202)patch(22-24)patch(115-129)patch(204-216)
android_sms_gateway/domain.py (1)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)
🪛 Ruff (0.13.3)
android_sms_gateway/http.py
109-109: Avoid specifying long messages outside the exception class
(TRY003)
123-123: Avoid specifying long messages outside the exception class
(TRY003)
199-199: Avoid specifying long messages outside the exception class
(TRY003)
215-215: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/domain.py
54-54: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
android_sms_gateway/domain.py (2)
74-144: LGTM! Well-structured message type classes.The
DataMessageandTextMessageclasses are cleanly designed with appropriate constructors and serialization methods. TheDataMessage.with_byteshelper is a practical addition for base64 encoding.
224-257: LGTM! Clean domain model additions.The
DeviceandErrorResponsedataclasses are well-structured with appropriatefrom_dictconstructors that correctly map JSON responses to Python objects.android_sms_gateway/http.py (3)
16-24: LGTM! Consistent protocol extension.The
putandpatchmethod signatures correctly mirror the existingpostmethod, maintaining consistency across theHttpClientprotocol.
101-127: LGTM! Consistent implementation.The
putandpatchmethods correctly follow the established pattern of the existingpostmethod, including session validation and response processing via_process_response.
191-221: LGTM! Consistent implementation.The
putandpatchmethods correctly follow the established pattern of the existingpostmethod with method chaining, including client validation and response handling.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_client.py (1)
72-78: Update docstring to mentionAPIError.This test (and the client behavior) now raises
errors.APIError, but the docstring still claims it raisesHTTPError. Please update the text so it documents the actual exception.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
android_sms_gateway/ahttp.py(8 hunks)android_sms_gateway/errors.py(1 hunks)android_sms_gateway/http.py(7 hunks)tests/test_client.py(3 hunks)tests/test_error_handling.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- android_sms_gateway/errors.py
🧰 Additional context used
🧬 Code graph analysis (4)
android_sms_gateway/http.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/ahttp.py (11)
put(21-23)put(118-131)put(222-233)patch(26-28)patch(133-146)patch(235-246)_process_response(77-92)_process_response(183-198)get(11-13)get(94-101)get(200-207)
tests/test_error_handling.py (3)
android_sms_gateway/errors.py (9)
APIError(4-15)BadRequestError(18-19)UnauthorizedError(22-23)ForbiddenError(26-27)NotFoundError(30-31)InternalServerError(34-35)ServiceUnavailableError(38-39)GatewayTimeoutError(42-43)error_from_status(57-72)android_sms_gateway/http.py (2)
_process_response(133-148)_process_response(176-191)android_sms_gateway/ahttp.py (2)
_process_response(77-92)_process_response(183-198)
tests/test_client.py (1)
android_sms_gateway/errors.py (1)
APIError(4-15)
android_sms_gateway/ahttp.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/http.py (11)
put(21-23)put(105-117)put(223-235)patch(26-28)patch(119-131)patch(237-249)_process_response(133-148)_process_response(176-191)get(11-13)get(75-81)get(193-199)
🪛 GitHub Actions: Python CI
tests/test_error_handling.py
[error] 4-4: ModuleNotFoundError: No module named 'httpx'. Ensure httpx is installed (add to Pipfile/requirements).
🪛 Ruff (0.13.3)
android_sms_gateway/http.py
113-113: Avoid specifying long messages outside the exception class
(TRY003)
127-127: Avoid specifying long messages outside the exception class
(TRY003)
148-148: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
191-191: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
197-197: Avoid specifying long messages outside the exception class
(TRY003)
231-231: Avoid specifying long messages outside the exception class
(TRY003)
245-245: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/ahttp.py
92-92: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
98-98: Avoid specifying long messages outside the exception class
(TRY003)
126-126: Avoid specifying long messages outside the exception class
(TRY003)
141-141: Avoid specifying long messages outside the exception class
(TRY003)
198-198: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
204-204: Avoid specifying long messages outside the exception class
(TRY003)
230-230: Avoid specifying long messages outside the exception class
(TRY003)
243-243: Avoid specifying long messages outside the exception class
(TRY003)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
android_sms_gateway/http.py (2)
184-188: Apply the same empty-body guard to the httpx client.The httpx variant has the identical failure mode:
response.json()on a successful but empty response raises and turns a success into an exception. Mirror the zero-length check here too.response.raise_for_status() - if response.status_code == 204: + if response.status_code == 204: return {} - return response.json() + if not response.content: + return {} + + return response.json()
136-140: Guard against empty success bodies before parsing JSON.Any 2xx without a body (e.g., 200/202 with
Content-Length: 0) now raisesValueErrorbecause we callresponse.json()unconditionally. That regresses DELETE/PUT/PATCH flows that legitimately return an empty body. Please short-circuit on zero-length content as well, not just on 204.response.raise_for_status() - if response.status_code == 204: + if response.status_code == 204: return {} - return response.json() + if not response.content: + return {} + + return response.json()android_sms_gateway/ahttp.py (2)
77-98: Guard async JSON parsing on empty/non-JSON bodies (aiohttp)await response.json() can raise ContentTypeError or JSON decode errors on 204/empty or non-JSON success responses. Also guard when extracting error bodies.
Apply this diff:
async def _process_response(self, response: aiohttp.ClientResponse) -> dict: - try: - response.raise_for_status() - if response.status == 204: - return {} - - return await response.json() - except aiohttp.ClientResponseError as e: + try: + response.raise_for_status() + if response.status == 204: + return {} + # Short‑circuit empty body + if response.content_length == 0: + return {} + try: + return await response.json() + except (aiohttp.ContentTypeError, ValueError): + # Treat empty or non‑JSON success payloads as empty dict + return {} + except aiohttp.ClientResponseError as e: # Extract error message from response if available - error_data = {} - try: - error_data = await response.json() - except ValueError: - # Response is not JSON - pass + error_data = {} + if response.status != 204: + try: + error_data = await response.json() + except (aiohttp.ContentTypeError, ValueError): + # Response is empty or not JSON + pass # Use the error mapping to create appropriate exception error_message = str(e) or "HTTP request failed" raise error_from_status( error_message, response.status, error_data ) from e
188-209: Guard async JSON parsing on empty/non-JSON bodies (httpx)response.json() raises on 204/empty or non-JSON success responses. Also avoid parsing error body if empty.
Apply this diff:
async def _process_response(self, response: httpx.Response) -> dict: - try: - response.raise_for_status() - if response.status_code == 204: - return {} - - return response.json() - except httpx.HTTPStatusError as e: + try: + response.raise_for_status() + if response.status_code == 204: + return {} + # Short‑circuit empty body + if not response.content or response.headers.get("Content-Length") == "0": + return {} + try: + return response.json() + except ValueError: + # Treat empty or non‑JSON success payloads as empty dict + return {} + except httpx.HTTPStatusError as e: # Extract error message from response if available - error_data = {} - try: - error_data = response.json() - except ValueError: - # Response is not JSON - pass + error_data = {} + if response.content: + try: + error_data = response.json() + except ValueError: + # Response is not JSON + pass # Use the error mapping to create appropriate exception error_message = str(e) or "HTTP request failed" raise error_from_status( error_message, response.status_code, error_data ) from e
🧹 Nitpick comments (1)
android_sms_gateway/domain.py (1)
43-45: Enforce ttl vs valid_until mutual exclusivityPrevent invalid states early by validating both aren’t set.
Apply this diff (after field declarations):
device_id: t.Optional[str] = None + def __post_init__(self): + # dataclasses with frozen=True can still validate + if self.ttl is not None and self.valid_until is not None: + raise ValueError("ttl and valid_until are mutually exclusive")Also applies to: 28-30
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/testing.yml(1 hunks)android_sms_gateway/ahttp.py(8 hunks)android_sms_gateway/domain.py(4 hunks)android_sms_gateway/http.py(7 hunks)tests/test_client.py(3 hunks)tests/test_error_handling.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/test_client.py (2)
android_sms_gateway/http.py (4)
RequestsHttpClient(56-153)get(11-13)get(75-81)get(203-209)android_sms_gateway/errors.py (1)
APIError(4-15)
android_sms_gateway/domain.py (1)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)
android_sms_gateway/ahttp.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/http.py (11)
put(21-23)put(105-117)put(233-245)patch(26-28)patch(119-131)patch(247-259)_process_response(133-153)_process_response(181-201)get(11-13)get(75-81)get(203-209)
android_sms_gateway/http.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/ahttp.py (11)
put(21-23)put(123-136)put(232-243)patch(26-28)patch(138-151)patch(245-256)_process_response(77-97)_process_response(188-208)get(11-13)get(99-106)get(210-217)
tests/test_error_handling.py (3)
android_sms_gateway/ahttp.py (4)
AiohttpAsyncHttpClient(58-160)HttpxAsyncHttpClient(169-265)_process_response(77-97)_process_response(188-208)android_sms_gateway/errors.py (9)
APIError(4-15)BadRequestError(18-19)ForbiddenError(26-27)GatewayTimeoutError(42-43)InternalServerError(34-35)NotFoundError(30-31)ServiceUnavailableError(38-39)UnauthorizedError(22-23)error_from_status(57-72)android_sms_gateway/http.py (4)
HttpxHttpClient(162-259)RequestsHttpClient(56-153)_process_response(133-153)_process_response(181-201)
🪛 GitHub Actions: Python CI
tests/test_error_handling.py
[error] 1-1: Async test functions are not supported in the current PyTest setup. Install a plugin like pytest-asyncio or anyio to run async tests.
🪛 Ruff (0.13.3)
android_sms_gateway/domain.py
55-55: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/ahttp.py
103-103: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
214-214: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Avoid specifying long messages outside the exception class
(TRY003)
253-253: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/http.py
113-113: Avoid specifying long messages outside the exception class
(TRY003)
127-127: Avoid specifying long messages outside the exception class
(TRY003)
207-207: Avoid specifying long messages outside the exception class
(TRY003)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
255-255: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
android_sms_gateway/ahttp.py (1)
20-28: Protocol additions (put/patch) look goodNew methods expand parity with sync client. Interfaces are consistent.
android_sms_gateway/domain.py (1)
65-78: Serializer upgrade looks solidEnum → value and datetime → ISO 8601; nested .asdict handled; None filtered. Good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
android_sms_gateway/domain.py (1)
53-59: Consider adding a docstring to document prioritization behavior.The property correctly returns content from either
text_messageordata_message, prioritizingtext_messagewhen both are present. Consider adding a docstring to clarify this prioritization:@property def content(self) -> str: + """Returns the message content. + + Returns text_message.text if present, otherwise data_message.data. + If both are present, text_message takes precedence. + + Returns: + str: The message content. + + Raises: + ValueError: If neither text_message nor data_message is set. + """ if self.text_message: return self.text_message.text if self.data_message: return self.data_message.data raise ValueError("Message has no content")Note: The TRY003 static analysis hint is a false positive.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android_sms_gateway/domain.py(4 hunks)tests/test_client.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_client.py
🧰 Additional context used
🧬 Code graph analysis (1)
android_sms_gateway/domain.py (1)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)
🪛 Ruff (0.14.0)
android_sms_gateway/domain.py
51-51: Avoid specifying long messages outside the exception class
(TRY003)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (6)
android_sms_gateway/domain.py (6)
1-5: LGTM! Past import issues addressed.The imports are now correct: unused
BytesIOhas been removed andenumhas been added to support enum serialization inMessage.asdict().
15-32: LGTM! Message class definition and docstring are well-structured.The keyword-only dataclass ensures clear API usage, and the docstring accurately reflects all field types including the corrected
booltypes for flags anddatetime.datetimeforvalid_until.
49-51: LGTM! Mutual exclusion validation is correct.The
__post_init__properly enforces thatttlandvalid_untilcannot both be set. The error message is concise and clear.Note: The static analysis hint (TRY003) is a false positive, as the message is appropriately short and specific to this context.
69-82: Excellent! Serialization logic fully addresses past concerns.The
_serializehelper correctly handles:
- Nested dataclasses (via
asdict())datetimeobjects (viaisoformat())Enumvalues (via.value)- Filtering out
NonefieldsThis implementation resolves the previous review comment about JSON serialization errors for enums and datetimes.
Based on past review comments.
85-155: LGTM! DataMessage and TextMessage are well-designed.Both classes are cleanly implemented as frozen dataclasses with appropriate serialization/deserialization methods:
DataMessage.with_bytes()provides convenient base64 encoding for binary payloads- Both
from_dict()methods enable straightforward deserialization- The fail-fast approach (no explicit key validation) will raise
KeyErrorfor missing fields, which is appropriate for this context
235-268: LGTM! Device and ErrorResponse classes are appropriately designed.Both frozen dataclasses provide clean representations of API entities with consistent
from_dict()constructors matching the pattern used throughout the file.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/test_domain.py (1)
146-267: Test updates are correct; expand coverage for new features.The parametrized tests correctly reflect the new
Messagestructure where:
- Constructor uses
text_message=TextMessage(text=...)instead ofmessage=...- Serialization produces
"textMessage": {"text": "..."}instead of"message": "..."However, significant gaps exist in test coverage for new domain features:
- DataMessage: Zero tests for the new
DataMessageclass despite being a major feature of this PR- Validation logic: No tests for the
ttl/valid_untilmutual exclusion constraint enforced in__post_init__- Content property: No tests for the
Message.contentproperty which has error handling logic- New fields: No tests for
device_idorvalid_untilfieldsAdd test cases covering these scenarios:
Messagewithdata_messageinstead oftext_messageMessagewith bothttlandvalid_until(should raiseValueError)contentproperty for both text and data messages, plus error case when neither is set- Serialization including
device_idandvalid_untilandroid_sms_gateway/ahttp.py (1)
188-208: Response handling addresses previous concerns.The implementation correctly checks for 204 status before attempting JSON parsing, which addresses the concern raised in previous reviews about empty responses causing errors. The httpx client's synchronous
response.json()is handled properly.For additional robustness, consider checking for empty response bodies beyond just 204 status:
async def _process_response(self, response: httpx.Response) -> dict: try: response.raise_for_status() - if response.status_code == 204: + if response.status_code == 204 or not response.content: return {} return response.json()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
android_sms_gateway/ahttp.py(8 hunks)tests/test_domain.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_domain.py (1)
android_sms_gateway/domain.py (3)
Webhook(196-232)Message(16-82)TextMessage(128-155)
android_sms_gateway/ahttp.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/http.py (11)
put(21-23)put(105-117)put(233-245)patch(26-28)patch(119-131)patch(247-259)_process_response(133-153)_process_response(181-201)get(11-13)get(75-81)get(203-209)
🪛 Ruff (0.14.0)
android_sms_gateway/ahttp.py
103-103: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
214-214: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Avoid specifying long messages outside the exception class
(TRY003)
253-253: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
tests/test_domain.py (1)
4-10: LGTM!The import of
TextMessagecorrectly reflects the new public API structure whereTextMessageis now a distinct domain class.android_sms_gateway/ahttp.py (4)
4-6: LGTM!The import of
error_from_statuscorrectly brings in the centralized error mapping functionality from the errors module.
20-28: LGTM!The protocol additions for
putandpatchmethods correctly define the async interface with proper type hints and abstract method decorators. The signatures align with the implementations in both client classes.
123-151: Implementation structure is correct.The
putandpatchmethods follow the established pattern and properly check session initialization before making requests. Once the critical issues in_process_responseare fixed (see comment on lines 77-97), these methods will work correctly.
232-256: LGTM!Both
putandpatchmethods are correctly implemented with proper client initialization checks and consistent patterns matching the existing request methods.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tests/test_error_handling.py (1)
3-4: Gate optional dependencies to prevent test suite crashes.This issue was flagged in a previous review and remains unresolved. The unconditional imports of
httpxandaiohttpcauseModuleNotFoundErrorin CI when those packages are not installed, preventing the entire test suite from running.Apply this diff to conditionally import and skip tests when optional backends are absent:
+# Try importing optional dependencies +pytest.importorskip("httpx") +pytest.importorskip("aiohttp") + import aiohttp import httpxAlternatively, move the imports into the test classes that need them and use
pytest.importorskip()at the class or method level.android_sms_gateway/ahttp.py (1)
77-97: Catchaiohttp.ContentTypeErrorfor non-JSON responses.While the
awaitkeywords are now correctly in place (resolving the past review), the code should also handleaiohttp.ContentTypeError, which is raised whenresponse.json()encounters a non-JSON content-type. Currently, onlyValueErroris caught.Apply this diff:
return await response.json() - except aiohttp.ClientResponseError as e: + except aiohttp.ContentTypeError: + # Response body is empty or not JSON + return {} + except aiohttp.ClientResponseError as e: # Extract error message from response if available error_data = {} try: error_data = await response.json() - except ValueError: + except (ValueError, aiohttp.ContentTypeError): # Response is not JSON or is empty pass
🧹 Nitpick comments (1)
tests/test_domain.py (1)
275-275: Remove redundant import.
DataMessageis already imported at the module level (line 11), making this local import unnecessary.Apply this diff:
- from android_sms_gateway.domain import DataMessage - data_msg = DataMessage(data="base64encodeddata", port=1234)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
android_sms_gateway/ahttp.py(8 hunks)tests/test_domain.py(8 hunks)tests/test_error_handling.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
android_sms_gateway/ahttp.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/http.py (11)
put(21-23)put(105-117)put(233-245)patch(26-28)patch(119-131)patch(247-259)_process_response(133-153)_process_response(181-201)get(11-13)get(75-81)get(203-209)
tests/test_domain.py (2)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)android_sms_gateway/domain.py (8)
Message(16-82)TextMessage(128-155)DataMessage(86-124)asdict(61-82)asdict(98-102)asdict(138-141)asdict(222-232)content(54-59)
tests/test_error_handling.py (3)
android_sms_gateway/ahttp.py (4)
AiohttpAsyncHttpClient(58-160)HttpxAsyncHttpClient(169-265)_process_response(77-97)_process_response(188-208)android_sms_gateway/errors.py (9)
APIError(4-15)BadRequestError(18-19)ForbiddenError(26-27)GatewayTimeoutError(42-43)InternalServerError(34-35)NotFoundError(30-31)ServiceUnavailableError(38-39)UnauthorizedError(22-23)error_from_status(57-72)android_sms_gateway/http.py (4)
HttpxHttpClient(162-259)RequestsHttpClient(56-153)_process_response(133-153)_process_response(181-201)
🪛 Ruff (0.14.0)
android_sms_gateway/ahttp.py
103-103: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
214-214: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Avoid specifying long messages outside the exception class
(TRY003)
253-253: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (13)
tests/test_error_handling.py (5)
23-79: LGTM!Comprehensive test coverage for
error_from_statusfactory function, verifying correct error class mapping, status code propagation, and response payload attachment across all defined HTTP error codes plus an unknown fallback case.
81-132: LGTM!Well-structured tests for
RequestsHttpClienterror handling with proper mock setup. Verifies correct error class instantiation, status code propagation, and response payload handling for both JSON and non-JSON error responses.
134-170: LGTM once import issue is resolved.Tests correctly validate
HttpxHttpClienterror handling with proper mock setup for httpx-specific exceptions. These tests will run once the optional dependency import issue (flagged earlier) is addressed.
172-216: LGTM once import issue is resolved.Correctly structured async tests for
AiohttpAsyncHttpClienterror handling. The use ofAsyncMockfor thejsonmethod is appropriate since aiohttp's response.json() is async. Tests will run once the optional dependency import issue is addressed.
218-255: LGTM once import issue is resolved.Async tests for
HttpxAsyncHttpClientare properly structured. Note that using regularMock(notAsyncMock) forjsonis correct here, as httpx's response.json() is synchronous even in the async client. Tests will run once the optional dependency import issue is addressed.tests/test_domain.py (3)
148-269: LGTM!The test updates correctly reflect the domain model changes. The migration from a flat
messagefield to nestedtextMessagestructure with proper camelCase serialization is handled accurately across all test cases.
272-324: LGTM!The new tests for
DataMessageand the mutual exclusivity validation betweenttlandvalid_untilare well-structured and comprehensive. The use ofpytest.raiseswith thematchparameter ensures precise error validation.
327-487: LGTM!Excellent test coverage for the new features:
- TTL and valid_until fields are tested both individually and in serialization
- The content property is thoroughly validated for both text and data messages, including error cases
- ISO format datetime serialization is correctly verified
- Comprehensive format tests ensure proper camelCase conversion and nested structure serialization
android_sms_gateway/ahttp.py (5)
4-6: LGTM!The import of
error_from_statusis correct and necessary for the new centralized error handling in_process_responsemethods.
20-28: LGTM!The addition of
putandpatchabstract methods to theAsyncHttpClientprotocol is correct and maintains consistency with the existing method signatures.
123-151: LGTM!The
putandpatchimplementations forAiohttpAsyncHttpClientfollow the established pattern and correctly delegate response handling to_process_response.
188-208: LGTM! (Past review concern is not applicable)The implementation correctly handles httpx responses. Note that
httpx.Response.json()is synchronous (not a coroutine), so the absence ofawaiton lines 194 and 199 is correct. The past review comment about missingawaitdoes not apply to httpx.The error handling appropriately catches
ValueError, which includesjson.JSONDecodeError, and the 204 status check prevents unnecessary JSON parsing for empty responses.
232-256: LGTM!The
putandpatchimplementations forHttpxAsyncHttpClientare correct and consistent with the established patterns. Both properly validate client initialization and delegate response processing to_process_response.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_domain.py (1)
311-385: Consider testing the edge case of both message types.The tests thoroughly cover:
- Mutual exclusivity of
ttlandvalid_until✓- Content property behavior for text, data, and neither ✓
- Proper error messages for invalid states ✓
However, consider adding a test for when both
text_messageanddata_messageare provided simultaneously. Based on thecontentproperty implementation indomain.py,text_messagetakes precedence, but this behavior is not explicitly tested.Add a test to verify the behavior:
def test_message_with_both_text_and_data_messages(): """Test behavior when both text_message and data_message are provided""" text_msg = TextMessage(text="Hello, world!") data_msg = DataMessage(data="base64encodeddata", port=1234) message = Message( phone_numbers=["123", "456"], text_message=text_msg, data_message=data_msg, ) # Verify text_message takes precedence in content property assert message.content == "Hello, world!" # Verify both are serialized serialized = message.asdict() assert "textMessage" in serialized assert "dataMessage" in serialized
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_domain.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_domain.py (2)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)android_sms_gateway/domain.py (8)
Message(16-82)TextMessage(128-155)DataMessage(86-124)asdict(61-82)asdict(98-102)asdict(138-141)asdict(222-232)content(54-59)
🔇 Additional comments (4)
tests/test_domain.py (4)
1-12: LGTM!The updated imports correctly reflect the new public API with
TextMessageandDataMessage, and thedatetimeimport is properly utilized in the new tests.
148-269: LGTM!The parametrized test properly reflects the API changes:
- Message constructor now uses
text_message=TextMessage(text=...)instead ofmessage=...- Expected serialization format correctly wraps text content under
"textMessage": {"text": "..."}All test cases are consistently updated and the test coverage remains comprehensive.
272-309: LGTM!The new tests properly verify
data_messagefunctionality:
- Creating a message with only
data_messageworks correctly- Serialization properly wraps data content under
"dataMessage"- The
device_idfield is correctly included in serialization
387-485: LGTM!The serialization tests provide excellent coverage:
- Individual field serialization (
device_id,valid_until,ttl) ✓- Correct camelCase conversion (
deviceId,validUntil) ✓- ISO format for datetime values ✓
- Comprehensive end-to-end serialization for both message types ✓
The comprehensive tests at the end (lines 428-485) serve as good integration tests that verify all fields work together correctly.
capcom6
force-pushed
the
client/add-new-api-support
branch
from
8d9c4a9 to
130ac2f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
android_sms_gateway/ahttp.py (1)
188-209: Apply the same empty-body robustness to the httpx client.The httpx
_process_responsemirrors the aiohttp version but should also handle non-JSON and empty responses beyond status 204. Unlike aiohttp, httpx'sresponse.json()raisesjson.JSONDecodeError(a subclass ofValueError) for empty bodies, so the currentValueErrorcatch may suffice. However, explicitly checking for empty content improves clarity and performance.Consider this defensive enhancement:
async def _process_response(self, response: httpx.Response) -> dict: try: response.raise_for_status() - if response.status_code == 204: + if response.status_code == 204 or not response.content: return {} return response.json() except httpx.HTTPStatusError as e: # Extract error message from response if available error_data = {} try: - error_data = response.json() + if response.content: + error_data = response.json() except ValueError: # Response is not JSON pass # Use the error mapping to create appropriate exception error_message = str(e) or "HTTP request failed" raise error_from_status( error_message, response.status_code, error_data ) from eandroid_sms_gateway/http.py (2)
133-153: LGTM with a minor robustness suggestion.The
_process_responsemethod correctly handles 204 responses and useserror_from_statusfor typed exceptions. The pattern matches the sync client requirements.For completeness, consider checking for empty content beyond just 204:
def _process_response(self, response: requests.Response) -> dict: try: response.raise_for_status() - if response.status_code == 204: + if response.status_code == 204 or not response.content: return {} return response.json() except requests.exceptions.HTTPError as e: # Extract error message from response if available error_data = {} try: - error_data = response.json() + if response.content: + error_data = response.json() except ValueError: # Response is not JSON pass # Use the error mapping to create appropriate exception error_message = str(e) or "HTTP request failed" raise error_from_status( error_message, response.status_code, error_data ) from e
181-201: LGTM with a minor robustness suggestion.The httpx
_process_responsecorrectly handles 204 and useserror_from_statusfor typed exceptions.Similar to the requests client, checking for empty content improves robustness:
def _process_response(self, response: httpx.Response) -> dict: try: response.raise_for_status() - if response.status_code == 204: + if response.status_code == 204 or not response.content: return {} return response.json() except httpx.HTTPStatusError as e: # Extract error message from response if available error_data = {} try: - error_data = response.json() + if response.content: + error_data = response.json() except ValueError: # Response is not JSON pass # Use the error mapping to create appropriate exception error_message = str(e) or "HTTP request failed" raise error_from_status( error_message, response.status_code, error_data ) from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Pipfile.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/testing.yml(1 hunks)Pipfile(2 hunks)android_sms_gateway/ahttp.py(8 hunks)android_sms_gateway/domain.py(4 hunks)android_sms_gateway/errors.py(1 hunks)android_sms_gateway/http.py(7 hunks)tests/test_client.py(3 hunks)tests/test_domain.py(8 hunks)tests/test_error_handling.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- android_sms_gateway/errors.py
- tests/test_client.py
- tests/test_error_handling.py
- Pipfile
- .github/workflows/testing.yml
🧰 Additional context used
🧬 Code graph analysis (4)
android_sms_gateway/http.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/ahttp.py (11)
put(21-23)put(123-136)put(232-243)patch(26-28)patch(138-151)patch(245-256)_process_response(77-97)_process_response(188-208)get(11-13)get(99-106)get(210-217)
android_sms_gateway/domain.py (1)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)
tests/test_domain.py (2)
android_sms_gateway/enums.py (1)
MessagePriority(33-46)android_sms_gateway/domain.py (8)
Message(16-82)TextMessage(128-155)DataMessage(86-124)asdict(61-82)asdict(98-102)asdict(138-141)asdict(222-232)content(54-59)
android_sms_gateway/ahttp.py (2)
android_sms_gateway/errors.py (1)
error_from_status(57-72)android_sms_gateway/http.py (11)
put(21-23)put(105-117)put(233-245)patch(26-28)patch(119-131)patch(247-259)_process_response(133-153)_process_response(181-201)get(11-13)get(75-81)get(203-209)
🪛 Ruff (0.14.0)
android_sms_gateway/http.py
113-113: Avoid specifying long messages outside the exception class
(TRY003)
127-127: Avoid specifying long messages outside the exception class
(TRY003)
207-207: Avoid specifying long messages outside the exception class
(TRY003)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
255-255: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/domain.py
51-51: Avoid specifying long messages outside the exception class
(TRY003)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
android_sms_gateway/ahttp.py
103-103: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Avoid specifying long messages outside the exception class
(TRY003)
214-214: Avoid specifying long messages outside the exception class
(TRY003)
240-240: Avoid specifying long messages outside the exception class
(TRY003)
253-253: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (24)
android_sms_gateway/ahttp.py (4)
4-6: LGTM!The import of
error_from_statusenables centralized error mapping from HTTP status codes to typed API exceptions, aligning with the error handling enhancements across the codebase.
20-28: LGTM!The addition of
putandpatchabstract methods to theAsyncHttpClientprotocol correctly extends the HTTP verb support with consistent signatures matching the existingpostmethod.
123-136: LGTM!The
putandpatchimplementations correctly delegate to_process_response, maintaining consistency with thegetandpostmethods. Session initialization checks and JSON payload handling are appropriate.Also applies to: 138-151
232-243: LGTM!The
putandpatchimplementations inHttpxAsyncHttpClientcorrectly useawait self._process_response(response), maintaining consistency with the other HTTP methods. Client initialization checks are in place.Also applies to: 245-256
tests/test_domain.py (7)
2-2: LGTM!The addition of
datetimeimport and the new domain entities (TextMessage,DataMessage) correctly supports the enhanced test coverage for the restructuredMessageclass.Also applies to: 10-11
148-269: LGTM!The parametrized tests correctly verify
Message.asdict()serialization with the newTextMessagewrapper. All test cases properly construct messages usingTextMessage(text=...)and validate the expected"textMessage": {"text": ...}nested structure. The coverage of optional fields (id,ttl,sim_number,priority) is comprehensive.
272-308: LGTM!The new tests for
DataMessagethoroughly validate:
- Construction with
dataandportattributes- Mutual exclusivity with
text_message(only one message type perMessage)- Serialization to
"dataMessage": {"data": ..., "port": ...}format- Inclusion of optional fields like
device_id
311-353: LGTM!The mutual exclusivity tests for
ttlandvalid_untilcorrectly verify:
__post_init__raisesValueErrorwhen both are set- Each field works independently
- Serialization includes only the non-
Nonefieldvalid_untilis serialized as ISO format string
356-384: LGTM!The
contentproperty tests comprehensively cover all scenarios:
- Deriving text from
text_message.text- Deriving data from
data_message.data- Raising
ValueErrorwhen neither is presentThis validates the content derivation logic across both message types.
387-425: LGTM!These tests validate that
device_id,valid_until, andttlare correctly serialized with proper naming (deviceId,validUntilin camelCase) and formatting (ISO string for datetime). The coverage ensures the serialization helper inMessage.asdict()handles these fields correctly.
428-485: LGTM!The comprehensive format tests validate the complete serialization structure for both
TextMessageandDataMessagepayloads, including all optional fields. These end-to-end tests ensure that the_serializehelper inMessage.asdict()correctly handles nested dataclasses, enums (MessagePriority), and all field combinations.android_sms_gateway/http.py (4)
4-6: LGTM!The import of
error_from_statusenables consistent error mapping in sync HTTP clients, aligning with the async implementations inahttp.py.
20-28: LGTM!The addition of
putandpatchabstract methods to the syncHttpClientprotocol correctly mirrors the async protocol extensions, maintaining API consistency across both client types.
105-117: LGTM!The
putandpatchimplementations inRequestsHttpClientcorrectly use_process_response()for consistent error handling and JSON processing. Session initialization checks are in place.Also applies to: 119-131
233-245: LGTM!The
putandpatchimplementations inHttpxHttpClientcorrectly delegate to_process_response, maintaining consistency with the other HTTP methods. Client initialization checks are present.Also applies to: 247-259
android_sms_gateway/domain.py (9)
1-5: LGTM!The import changes correctly address previous feedback:
- Removed unused
io.BytesIO- Added
datetimefor thevalid_untilfield- Added
enumfor serializingMessagePriorityvalues
15-47: LGTM!The restructured
Messageclass with keyword-only arguments (kw_only=True) provides a clear, extensible API. The split intotext_messageanddata_messageproperly represents the union type for message content, while the addition ofdevice_id,priority, andvalid_untilenhances message control. The updated docstring accurately reflects all fields with correct types.
49-51: LGTM!The
__post_init__validation correctly enforces mutual exclusivity betweenttlandvalid_until, preventing conflicting expiration specifications. The error message is clear and actionable.
53-59: LGTM!The
contentproperty elegantly provides a unified interface for accessing message content from eithertext_messageordata_message, with appropriate error handling when neither is present. This simplifies client code that doesn't need to distinguish between message types.
61-82: LGTM!The enhanced
asdict()serialization with the internal_serializehelper correctly addresses previous issues by:
- Recursively calling
asdict()on nested dataclasses (TextMessage,DataMessage)- Converting
datetimeobjects to ISO 8601 format- Extracting enum values for JSON compatibility
- Filtering out
NonefieldsThis ensures the serialized output is fully JSON-compatible and includes only meaningful data.
85-124: LGTM!The
DataMessagedataclass is well-designed with:
- Clear attributes for base64-encoded data and destination port
asdict()for serializationwith_bytes()class method for convenient construction from raw bytesfrom_dict()for deserialization from API responsesThis provides a complete API for SMS data messages with proper encoding.
127-155: LGTM!The
TextMessagedataclass provides a clean wrapper for text content with symmetric serialization/deserialization methods. This aligns well with theDataMessagedesign, maintaining consistency across message types.
235-250: LGTM!The
Devicedataclass introduces device representation withfrom_dict()for API response parsing. The structure is minimal but complete for device identification.
253-268: LGTM!The
ErrorResponsedataclass provides structured error information from API responses withcodeandmessagefields. Thefrom_dict()method enables clean parsing of error payloads for typed error handling.
Summary by CodeRabbit
New Features
Improvements
Error Handling
Tests