Skip to content

feat: migrate HTTP client to generated ros2-medkit-client#13

Merged
bburda merged 4 commits intomainfrom
feat/generated-client-migration
Mar 30, 2026
Merged

feat: migrate HTTP client to generated ros2-medkit-client#13
bburda merged 4 commits intomainfrom
feat/generated-client-migration

Conversation

@bburda
Copy link
Copy Markdown
Contributor

@bburda bburda commented Mar 30, 2026

Pull Request

Summary

Replace hand-written httpx client (~1078 lines) with a wrapper around the generated ros2-medkit-client package. SovdClient interface preserved - zero changes needed in mcp_app.py tool dispatcher.

Key changes:

  • client.py delegates to MedkitClient via _call() with _ENTITY_FUNC_MAP dispatch for entity-type-polymorphic endpoints
  • Structured SOVD error codes preserved in SovdClientError messages
  • asyncio.Lock on lazy client init for concurrent safety
  • SSRF protection on get_bulk_data_info (was missing vs download)
  • URL-encoded path segments in raw request fallbacks (fault snapshots)
  • Extracted _httpx_client(), _extract_filename(), _validate_relative_uri() helpers

Also fixes:

  • update_execution dispatcher used args.request_data instead of args.update_data (pre-existing bug)
  • FaultItem model accepts fault_code alias from generated client
  • Fault formatting fallbacks check both camelCase and snake_case keys

Net -209 lines (746 added, 955 removed).


Issue


Type

  • Bug fix
  • New feature
  • Breaking change
  • Documentation only

Testing

  • 111 tests pass (poetry run python run_tests.py -v)
  • Mock responses updated to match OpenAPI schema (snake_case fields, required fields)
  • Lint clean (ruff check), format clean (ruff format --check)
  • Deep review performed with 5 parallel agents (code quality, tests, docs, security, UX)

Checklist

  • Breaking changes are clearly described (and announced in docs / changelog if needed)
  • Linting passes (poetry run ruff check src/ tests/)
  • Formatting passes (poetry run ruff format --check src/ tests/)
  • Type checking passes (poetry run mypy src/)
  • Tests pass (poetry run python run_tests.py)
  • Docs were updated if behavior or public API changed

Replace hand-written httpx client (~1078 lines) with a wrapper around
the generated ros2-medkit-client package. SovdClient interface preserved
- zero changes needed in mcp_app.py tool dispatcher.

Key changes:
- client.py: SovdClient delegates to MedkitClient via _call() with
  _ENTITY_FUNC_MAP dispatch for entity-type-polymorphic endpoints
- Structured SOVD error codes preserved in SovdClientError messages
- asyncio.Lock on lazy client init for concurrent safety
- SSRF protection on get_bulk_data_info (was missing vs download)
- URL-encoded path segments in raw request fallbacks
- Extracted _httpx_client(), _extract_filename(), _validate_relative_uri()

Also fixes:
- update_execution dispatcher used args.request_data instead of
  args.update_data (pre-existing bug, AttributeError at runtime)
- FaultItem model accepts fault_code alias from generated client
- Fault formatting fallbacks check both camelCase and snake_case keys

Tests updated: mock responses aligned with OpenAPI schema (snake_case
field names, required fields). 111 tests pass.

Closes #11
Copilot AI review requested due to automatic review settings March 30, 2026 15:31
@bburda bburda self-assigned this Mar 30, 2026
@bburda bburda requested a review from mfaferek93 March 30, 2026 15:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the MCP server’s SOVD HTTP integration from a large hand-written httpx client to a thin compatibility wrapper around the generated ros2-medkit-client, aiming to preserve the existing SovdClient interface consumed by mcp_app.py.

Changes:

  • Replaced the hand-written async HTTP client with a MedkitClient-backed SovdClient wrapper, including entity-type dispatch via _ENTITY_FUNC_MAP.
  • Updated fault modeling/formatting to better handle snake_case responses from the generated client.
  • Updated tests and mocks to match the generated client’s response shapes; added the ros2-medkit-client dependency.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/ros2_medkit_mcp/client.py Introduces the new MedkitClient-based wrapper, entity dispatch mapping, and raw fallbacks (snapshots/bulk-data).
src/ros2_medkit_mcp/mcp_app.py Adjusts fault formatting fallbacks and fixes update_execution argument wiring.
src/ros2_medkit_mcp/models.py Updates FaultItem to accept the generated client’s fault_code naming.
tests/test_tools.py Updates mocked responses and assertions to the new schema/behavior.
tests/test_mcp_app.py Updates integration-style tests to the new response shapes.
tests/test_bulkdata_tools.py Updates bulk-data test fixtures to new required fields / key casing.
pyproject.toml Adds ros2-medkit-client dependency via a GitHub release wheel URL.
poetry.lock Locks the new dependency and transitive additions.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

- _validate_relative_uri raises SovdClientError instead of ValueError
- _raw_request handles JSON decode failures
- _call catches ValueError/KeyError from generated client parsing
- Narrowed test_non_json_response to assert SovdClientError only
- test_get_version_error asserts error code in message
- Added comment about URL dependency in pyproject.toml
- Auto-wrap body dicts as generated model instances via _wrap_body_dict()
  (fixes runtime crash in set_configuration and all body-accepting endpoints)
- Add import-time validation of _ENTITY_FUNC_MAP function references
  (fails fast on generator rename instead of silent runtime AttributeError)
Copilot AI review requested due to automatic review settings March 30, 2026 19:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

@mfaferek93 mfaferek93 self-requested a review March 30, 2026 20:04
@bburda bburda merged commit b8c53d5 into main Mar 30, 2026
7 checks passed
@bburda bburda deleted the feat/generated-client-migration branch March 30, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate to generated Python client from ros2_medkit_clients

3 participants