Repository navigation
feat(diagnostics): expose write protocol health and stats - #28
Conversation
xe-nvdk
left a comment
There was a problem hiding this comment.
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".
|
#29 just landed on main: the 0.2.0 parity work, including a new
|
992b967 to
3e5cee8
Compare
|
Thanks for the review. I’ve rebased this branch onto the current
|
3e5cee8 to
0256345
Compare
xe-nvdk
left a comment
There was a problem hiding this comment.
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:
- CI (I approved the workflow run):
ruff formatwants to reformatsrc/arc_client/async_client.py. Runningruff format src testsfixes it. msgpack_spec(): Arc's spec still says authentication is viax-api-keyand "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.
0256345 to
242e516
Compare
|
Thanks for the live validation. I’ve addressed the two remaining items and rebased the branch onto current upstream
The existing raw-JSON contract documentation and read-only scope remain unchanged; the global flush endpoint is still intentionally excluded. Static |
# Conflicts: # src/arc_client/async_client.py # src/arc_client/client.py # src/arc_client/management/__init__.py
…agnostics (Basekick-Labs#19) The four read-only routes now live on the existing diagnostics client, next to metrics and logs, instead of a second top-level client.write_diagnostics: client.diagnostics.line_protocol_health() client.diagnostics.line_protocol_stats() client.diagnostics.msgpack_spec() client.diagnostics.msgpack_stats() They reuse DiagnosticsClient._json, so a non-object response raises ArcError like the rest of the client. Docstrings record what was checked against Arc: - All four take a token of any permission (a read-only token works) and exist from Arc 25.12.1, not 26.09.3. - Line Protocol health is a constant "healthy"; write_readiness() is the real signal. - The msgpack spec's authentication section is correct from 27.01.1; 26.09.3 and earlier say auth is not implemented. Tests now cover a 401 (Arc's real missing-token response) instead of a 403 labelled "Authentication required", and a non-object response. README and CHANGELOG entries moved under the diagnostics sections.
|
Thanks @lecodev-26. We took this over to finish it, merged API placement. The four methods now live on the existing client.diagnostics.line_protocol_health()
client.diagnostics.line_protocol_stats()
client.diagnostics.msgpack_spec()
client.diagnostics.msgpack_stats()They reuse that client's Corrections, checked against Arc's source and a live 27.01.1 server with auth on:
Checks: 1121 unit tests, ruff and mypy (as CI runs it) pass. Live against Arc: each method with admin, read-only, invalid and missing tokens, sync and async, and Line Protocol stats counting a 2-line write exactly. README and CHANGELOG entries moved under the diagnostics sections. Marking ready for review. |
Summary
client.write_diagnosticsmethods for Line Protocol health and statistics, plus MessagePack protocol specification and decoder/buffer statistics.Scope
This is a read-only, independently reviewable part of #19. The administrative
POST /api/v1/write/line-protocol/flushendpoint 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
git diff --checkand stagedgit diff --cached --checkpassed.Part of #19.