Skip to content

feat(diagnostics): expose write protocol health and stats - #28

Draft
lecodev-26 wants to merge 2 commits into
Basekick-Labs:mainfrom
lecodev-26:feat/write-protocol-diagnostics-19
Draft

lecodev-26 wants to merge 2 commits into
Basekick-Labs:mainfrom
lecodev-26:feat/write-protocol-diagnostics-19

Conversation

@lecodev-26

Copy link
Copy Markdown

Summary

  • Add synchronous and asynchronous client.write_diagnostics methods for Line Protocol health and statistics, plus MessagePack protocol specification and decoder/buffer statistics.
  • Preserve the server's JSON payloads and existing typed HTTP errors, including permission and server failures.
  • Document authentication, usage, and server compatibility; add HTTP contract tests for both client modes.

Scope

This is a read-only, independently reviewable part of #19. The administrative POST /api/v1/write/line-protocol/flush endpoint is intentionally excluded: it forces a global flush across shards, is admin-gated, and is operationally heavier than diagnostics. It can be evaluated as a separate change with its own explicit safety contract.

The four included GET routes are present in Arc v26.09.3 (e83bb1e) and current main (a63f4a8). They require an authenticated token; server authorization and error responses are not bypassed by the SDK.

Verification

  • Static review: git diff --check and staged git diff --cached --check passed.
  • Added sync/async HTTP contract tests for all four routes, bearer authentication, 403 permission errors, 503 server errors, and response-body preservation.
  • Tests were added but not run locally. No tests, builds, compilation, benchmarks, or equivalent dynamic processes were run in Termux. Please rely on CI for dynamic validation.
  • Manual Test: not performed.
  • IA Test: not performed.

Part of #19.

@xe-nvdk xe-nvdk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Checked out and ran locally: 436 passed / 9 skipped, mypy clean, ruff check clean, but ruff format --check fails on src/arc_client/async_client.py — CI will go red once the run is approved. Please run uv run ruff format ..

Routes verified against Arc (internal/api/lineprotocol.go:106-107, msgpack.go:187-188): paths and the auth-only gate match. Leaving /line-protocol/flush out of a read-only interface is a sensible scope cut; it'll be added separately for the 0.2.0 parity work.

Notes:

  • The sub-client returns raw dicts while the other management clients return Pydantic models. Fine for operator diagnostics whose shape Arc doesn't promise, but please say so in the docstrings.
  • #27 touches the same lines in client.py, async_client.py, README and CHANGELOG; whichever lands second needs a rebase.
  • The README says "available on Arc v26.09.3 and current main" — SDK 0.2.0 will ship with Arc 2027.01.1, so phrase it as "Arc 26.09.3 and later".

@xe-nvdk

xe-nvdk commented Nov 2, 2026

Copy link
Copy Markdown
Member

#29 just landed on main: the 0.2.0 parity work, including a new client.diagnostics and changes to client.py, async_client.py, README and CHANGELOG. Please rebase onto main and resolve the conflicts. Things to know:

  • tests/conftest.py now has an autouse fixture that isolates the installation-id file, so nothing is needed on your side for it.
  • Typed ArcErrors should pass through unchanged, the way the query and write clients now do. Wrapping is only for non-Arc exceptions, which your _json helper already does.
  • The CHANGELOG Added section has grown; put your entry next to the other operator surfaces.
  • After rebasing, please run uv run pytest tests/unit -q, uv run ruff check ., uv run ruff format --check . and uv run mypy src/arc_client --ignore-missing-imports locally. The earlier review found one lint failure in each PR, which CI would have caught. If you can't run them, say so and I'll run them on the rebased branch.

lecodev-26 commented Nov 2, 2026 •

Copy link
Copy Markdown
Author

Thanks for the review. I’ve rebased this branch onto the current main (including #29) and addressed the requested updates:

  • Documented in both sync and async docstrings that these methods return raw, server-defined JSON because the diagnostic response shape is not a stable contract.
  • Updated the README wording to “Arc 26.09.3 and later”.
  • Kept the interface read-only and left the global administrative flush endpoint out of scope.
  • Resolved the overlapping README/CHANGELOG/client changes from feat(sdk): 0.2.0 parity with Arc main + opt-out installation telemetry #29.
  • Adjusted the single-symbol async diagnostics import to the formatter’s compact form in async_client.py.

git diff --check and conflict-marker/line-length checks pass. I have not run tests or builds in Termux; uv/ruff are not installed in that environment, so I could not execute the requested local test/lint/format/mypy commands. Please run them in CI or on your side.

@lecodev-26
lecodev-26 force-pushed the feat/write-protocol-diagnostics-19 branch from 3e5cee8 to 0256345 Compare November 2, 2026 17:41

@xe-nvdk xe-nvdk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I merged this onto current main and ran it against an Arc main build (92a1f9d1). All four routes return what the docstrings say. A read-only token can call them, a request without a token gets ArcAuthenticationError, and the stats counters moved after a write. The unit tests pass (1091).

Two small things:

  1. CI (I approved the workflow run): ruff format wants to reformat src/arc_client/async_client.py. Running ruff format src tests fixes it.
  2. msgpack_spec(): Arc's spec still says authentication is via x-api-key and "not yet implemented", which is stale (Basekick-Labs/arc#1434). A one-line docstring note will keep users from trusting that section until Arc fixes it.

On leaving out the flush endpoint: agreed, it's not a diagnostic. We'll add it on the maintainer side as a write-path method (client.write...), since our live test suites already need it.

This will need a small rebase if #27 lands first: both PRs add lines at the same spots in client.py, async_client.py and management/__init__.py. Ready to merge once formatting is fixed.

@lecodev-26
lecodev-26 force-pushed the feat/write-protocol-diagnostics-19 branch from 0256345 to 242e516 Compare November 2, 2026 21:25

Copy link
Copy Markdown
Author

Thanks for the live validation. I’ve addressed the two remaining items and rebased the branch onto current upstream main (358f9e9, including #32):

  • Reformatted the lazy async diagnostics import in async_client.py into the multiline form Ruff expects.
  • Added the same warning to the sync and async msgpack_spec() docstrings: the current Arc response has stale authentication metadata (“not yet implemented” and only x-api-key), so callers should not rely on that section until the server-side spec is corrected (#1434).

The existing raw-JSON contract documentation and read-only scope remain unchanged; the global flush endpoint is still intentionally excluded. Static git diff --check and changed-file line-length checks pass. I have not run tests, builds, compilation, benchmarks, or equivalent dynamic processes in Termux; ruff and uv are not installed there. Please rerun the CI checks on this updated head.

This branch has not been deployed

No deployments
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.

2 participants