refactor: extract a shared _dispatch helper for the unary senders

[craigmcchesney]

Closes #14.

The 18 unary _send_* methods across the five service clients each wrote out the same three-tier error-handling block — gRPC exception, business exceptionalResult, unexpected exception — differing only in the stub method, the success oneof field, the result class, and the operation name. They now delegate to one _dispatch() on ServiceApiClientBase.

Net −282 lines of client code, and each of #6's 10 new senders costs ~6 lines instead of ~40. Plan: plan/tickets/14/plan.md.

Triage notes

The ticket's premise held, and its corrected count of 18 unary senders is right (20 _send_* total, minus the 2 server-streaming). Three things it did not cover, all preserved rather than dropped:

• Success/entry logs are not uniform (plan T3/T4). Every sender logs a bespoke INFO line, and seven report a count read off the response ("QueryPvMetadata returned %d records", "Successfully saved %d sample status(es)"). The ticket's sketched signature has nowhere to put them, so a literal extraction would have silently dropped all 18 — and since no test asserts on log content (plan T6), nothing would have flagged it. _dispatch takes optional request_log / success_log callables; senders that logged a bare "Calling <op> API" pass nothing and get the byte-identical default.

• IngestionClient alone guarded e.code() (plan T5) against a bare RpcError — the shape its own test raises, on which code() genuinely does not resolve. That guard is now shared behavior: it adds the code to the other 17 senders' logs where it resolves, and changes no returned message.

• The two server-streaming senders stay hand-written. They yield one result per streamed message, error results included for the public iter_* wrapper to convert — a different contract from returning a single result.

Verification

• Error-message text is byte-identical throughout (about 40 assertions match on it with assertIn), so the existing suite covers the refactored path unedited: 406 passing before, 406 after.
• Log output diffed against main sender-by-sender, by rendering each sender's output in all six scenarios: all 18 emit from the same logger, with exactly two deliberate differences and no others:
  • the gRPC-error log now appends (code: ...) where the code resolves (the IngestionClient guard, generalized);
  • the business-error warning names the operation with the raw camelCase op_name (savePvMetadata API returned business error) instead of the capitalized form the hand-written senders used on that one line, while already using camelCase in their other two error logs. That inconsistency was dropped deliberately rather than preserved; a test now pins the format. Log text only — returned messages are unchanged.
• New tests/unit/test_service_api_client_base.py covers _dispatch's own branches, including the code() guard no per-client test reaches and the business-error log format — 416 total.
• ruff check / ruff format --check clean; cookbook snippet checker passes (78 snippets).

CLAUDE.md documents the pattern so #6 and later work write senders in the collapsed form.

🤖 Generated with Claude Code

https://claude.ai/code/session_011miYLGXSFNyPJWhBJr2wLK

[Copilot]

🟡 Changes recommended

A claimed “byte-identical” logging invariant is not fully preserved (business-error log prefix case differs) and there’s a docstring typo in a touched hunk.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Refactors the Python gRPC client library to centralize the repeated three-tier unary RPC error-handling logic into a single ServiceApiClientBase._dispatch() helper, reducing duplication across service clients while keeping the existing result/exception contract.

Changes:

• Added ServiceApiClientBase._dispatch() to handle unary stub invocation, business errors (exceptionalResult), gRPC errors, and unexpected exceptions.
• Updated all 18 unary _send_* methods across clients to delegate to _dispatch() (streaming senders remain bespoke).
• Added focused unit tests for _dispatch() and documented the new pattern in CLAUDE.md, plus a ticket plan under plan/tickets/14/.

File summaries

File	Description
tests/unit/test_service_api_client_base.py	Adds direct unit coverage for _dispatch() branches, including the code() guard and default/custom logging hooks.
src/dp_python_lib/client/service_api_client_base.py	Introduces _dispatch() helper to standardize unary RPC dispatch + error handling and logging.
src/dp_python_lib/client/sample_status_client.py	Refactors unary senders to delegate to _dispatch() with preserved request/success logs.
src/dp_python_lib/client/query_client.py	Refactors unary sender to delegate to _dispatch().
src/dp_python_lib/client/pv_metadata_client.py	Refactors unary senders to delegate to _dispatch() with preserved request/success logs.
src/dp_python_lib/client/machine_config_client.py	Refactors unary senders to delegate to _dispatch() with preserved request/success logs.
src/dp_python_lib/client/ingestion_client.py	Refactors unary sender to delegate to _dispatch().
plan/tickets/14/plan.md	Adds a point-in-time plan/record for the refactor’s intent and constraints.
CLAUDE.md	Documents the _dispatch() pattern and clarifies unary vs streaming sender expectations.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.