Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new async Python client package (ros2-medkit-client) intended to wrap an OpenAPI-generated client with higher-level ergonomics (context manager, error bridging) and SSE streaming support, plus CI to generate/test/publish.
Changes:
- Introduces
MedkitClient,MedkitErrorhierarchy, and SSE streaming (SseStream/StreamHelpers) wrapper layer. - Adds Python unit tests covering URL normalization, call() error bridging, SSE parsing/reconnect behavior, and stream URL construction.
- Adds Python CI workflow and repo README guidance for generation/installation/usage.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents repo structure and usage for TS + Python clients |
| .github/workflows/python-ci.yml | CI to generate client, run tests/lint/format, and publish to GitHub Packages |
| clients/python/pyproject.toml | Python package metadata, deps, and tooling config |
| clients/python/src/ros2_medkit_client/client.py | Async context manager client + error-bridged call() |
| clients/python/src/ros2_medkit_client/sse.py | SSE parsing + reconnect/backoff logic built on httpx streaming |
| clients/python/src/ros2_medkit_client/streams.py | Convenience constructors for gateway SSE endpoints |
| clients/python/src/ros2_medkit_client/errors.py | Structured exception types mapping to GenericError |
| clients/python/src/ros2_medkit_client/api/*.py | Re-export modules intended to hide generated internals |
| clients/python/tests/* | Unit test coverage for wrappers and SSE behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mfaferek93
reviewed
Mar 24, 2026
mfaferek93
previously approved these changes
Mar 24, 2026
Add pyproject.toml with hatchling build, httpx + attrs deps, pytest-asyncio + respx + ruff dev deps.
MedkitError with code/error_code and details/parameters dual names. MedkitConnectionError and MedkitTimeoutError subclasses.
Async context manager configuring generated Client. normalize_base_url handles missing protocol and /api/v1. Generated Client lifecycle managed via __aenter__/__aexit__.
Async iterator over SSE events. Exponential backoff reconnect with Last-Event-ID, server retry delay (clamped), buffer limits (1MB/256KB), CRLF handling, cancellable sleep. Retry/id applied regardless of data.
3 helpers cover 8 SSE endpoints. Entity IDs URL-encoded via urllib.parse.quote. Auth headers forwarded to SSE connections.
Public API exports MedkitClient, MedkitError, SseStream, SseEvent. API re-export modules with explicit submodule imports for all 14 API groups. Add python-dateutil dependency required by generated models.
Build, test, lint on PR. Publish to GitHub Packages on main push.
- Catch only httpx transport errors and OSError (not all Exception) - Wrap into MedkitConnectionError/MedkitTimeoutError on max retries - Set SSE httpx timeout: connect=10s, read=None (long-lived streams) - Add tests for timeout wrapping, close during sleep, non-JSON errors
- MedkitClient.call() wraps generated API functions, raises MedkitError on GenericError responses and None returns - Type http property as Any for IDE support - Fix CI publish to fail properly (no silent error suppression) - Add py.typed marker for PEP 561 - Add tests for property guards, auth propagation, call() method
- Restore original README content (structure table, spec docs, TS client) - Add Python client section with call() examples (not misleading try/except) - Use twine --skip-existing instead of broken version check - Show both call() and raw API usage patterns
CRLF is stripped in _connect_and_stream before calling parse_sse_line. Remove redundant strip in parse_sse_line, update test to match.
Improves IDE support - consumers can subscript event.data without casts.
- Distinguish SSE-only mode from uninitialized client in error messages - Document attrs and python-dateutil as generated code dependencies - Add build instructions comment for _generated symlink
9d10b76 to
1327349
Compare
mfaferek93
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Add
ros2-medkit-client- an async Python client for the ros2_medkit gateway. Wraps the OpenAPI 3.1.0 spec with SSE streaming via httpx, error bridging (client.call()raisesMedkitErroron GenericError responses), URL normalization, and reconnect logic.Key components:
MedkitClientasync context manager with URL normalization, JWT auth, generated Client lifecycleMedkitErrorexception hierarchy withMedkitConnectionError/MedkitTimeoutErrorsubclassesSseStreamclass - httpx.stream() based SSE parser with exponential backoff reconnect, Last-Event-ID, server retry delay (clamped), buffer limits (1MB/256KB), CRLF handling, cancellable sleepclient.call()error bridging - converts GenericError returns into MedkitError exceptionsfaults,trigger_events,subscription_events) covering 8 SSE endpointsIssue
Type
Testing
90 unit tests with mocked HTTP (pytest + respx + pytest-asyncio):
test_errors.py(14) - MedkitError hierarchy, dual field names, aliasestest_client.py(21) - URL normalization, context manager, auth propagation, call() error bridgingtest_sse.py(34) - SSE parsing, reconnect, retry delay, Last-Event-ID, auth failure, buffer overflow, data overflow, chunked data, CRLF, close during sleep, timeout wrappingtest_streams.py(21) - URL construction, entity types, URL encoding, auth headers, retry paramsChecklist