Skip to content

feat: add runtime_status and is_ready - #173

Open
claudespice wants to merge 4 commits into
spiceai:trunkfrom
claudespice:feat/runtime-status
Open

feat: add runtime_status and is_ready#173
claudespice wants to merge 4 commits into
spiceai:trunkfrom
claudespice:feat/runtime-status

Conversation

@claudespice

Copy link
Copy Markdown

What

Adds two runtime health methods to Client:

  • runtime_status()list[ConnectionDetails], wrapping GET /v1/status. One entry
    per runtime connection (http, flight, metrics, opentelemetry) with that
    component's endpoint and ComponentStatus.
  • is_ready()bool, wrapping GET /v1/ready.

ComponentStatus and ConnectionDetails are exported from spicepy.

Why

Neither endpoint was reachable from this SDK, so a Python user waiting for a runtime
to come up — or diagnosing one that came up wrong — had to hand-roll requests calls
alongside the client they already had configured.

/v1/ready collapses the runtime to one boolean; /v1/status says which component
is not ready, which is the difference between "not up yet" and "Flight is failing".
Neither requires cluster mode or extra configuration, so both work on a default
spice run.

Three details worth a reviewer's eye:

  • /v1/ready answers text/plain and uses 503 to mean "not ready", so it cannot go
    through send_request, which calls raise_for_status() and then .json(). This adds
    HttpRequests.send_request_raw for endpoints where the status code carries the
    meaning or the body is not JSON.
  • is_ready() returns False for the runtime's 503 and raises SpiceAIError for
    anything else, so "not ready" and "could not ask" stay distinct.
  • ComponentStatus.from_value returns an unrecognized status as a plain str rather
    than raising, so a status variant added by a future runtime does not break parsing.
    An unknown status is never treated as ready.

Part of aligning runtime health/status coverage across the Spice SDKs.

Verification

  • make test-unit — 404 passed (up from 379; 25 new tests)
  • make type-check (mypy) — success, no issues in 11 source files
  • ruff check on the new files — clean; whole-repo error count unchanged from
    trunk (2 pre-existing PLR0917 in _client.py, untouched here)
  • ruff format --check on the new files — clean
  • pylint — 9.27/10, above the 8.0 gate
  • Integration tests (make test-integration) — not run; they require a live
    runtime, which is not available in this environment.

Note on the 8 remaining test-unit failures: they are the pre-existing
adbc_driver_manager parameterized-query tests in tests/test_main.py, and they fail
identically on unmodified trunk here. This change adds no new failures.

Scoped deliberately to additions only — I avoided running make format, which with the
current ruff>=0.15.12 would reformat 15 files unrelated to this change.

Adds Client.runtime_status (GET /v1/status), reporting per-component state for
http, flight, metrics and opentelemetry, and Client.is_ready (GET /v1/ready)
for the boolean case. Neither was reachable from the SDK.

/v1/ready answers with text/plain and uses 503 to mean 'not ready', so it needs
a raw response rather than send_request's raise_for_status + json decode; adds
HttpRequests.send_request_raw for that.

ComponentStatus preserves an unrecognized status as a plain string so a status
added by a newer runtime does not raise.
@lukekim
lukekim requested a review from Copilot July 26, 2026 01:05
@lukekim lukekim self-assigned this Jul 26, 2026
@lukekim lukekim added the enhancement New feature or request label Jul 26, 2026
@lukekim lukekim assigned claudespice and unassigned lukekim Jul 26, 2026
@lukekim lukekim added this to the v4.0.0 milestone Jul 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds runtime health/status support to the Python SDK by introducing typed status models and exposing two new Client methods for probing runtime readiness and per-component status.

Changes:

  • Add Client.runtime_status() (wraps GET /v1/status) returning list[ConnectionDetails].
  • Add Client.is_ready() (wraps GET /v1/ready) returning bool, backed by new HttpRequests.send_request_raw().
  • Export ComponentStatus and ConnectionDetails from the top-level package and document usage in the README, with unit tests covering the new behavior.

Reviewed changes

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

Show a summary per file
File Description
tests/test_status.py Adds unit tests for status parsing and the new Client health methods.
tests/test_http.py Adds unit tests for the new raw HTTP request helper.
spicepy/_status.py Introduces ComponentStatus and ConnectionDetails models for /v1/status.
spicepy/_http.py Adds HttpRequests.send_request_raw() to return a raw Response without raising/JSON decoding.
spicepy/_client.py Adds Client.runtime_status() and Client.is_ready() using the HTTP helpers.
spicepy/init.py Re-exports new status types (ComponentStatus, ConnectionDetails).
README.md Documents runtime health/status usage patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread spicepy/_http.py Outdated
Comment thread spicepy/_client.py
black --check runs at its default 88-column width, while ruff is configured
for 120, so three assertion call sites that satisfied ruff still failed the
black gate. Wrapping is mechanical — the arguments are unchanged.
auto-merge was automatically disabled July 27, 2026 19:12

Head branch was pushed to by a user without write access

@claudespice

Copy link
Copy Markdown
Author

Triaged the red checks on this PR — they had two independent causes, only one of which was mine.

Mine, now fixed (420e77a): Lint with black flagged tests/test_status.py. black --check runs at its default 88-column width while ruff here is configured for 120, so three assertion call sites that satisfied ruff still failed the black gate. Wrapped them; the arguments are unchanged, and black --check spicepy tests is now clean locally against black 26.5.1 (the version CI resolves).

Not mine — these are the repo-wide breakage #176 already fixes, and they fail identically on #155, #159 and the other open PRs:

  • Lint with ruffPLR0917 Too many positional arguments at spicepy/_client.py:241 and :334. Both are __init__ signatures this PR never touches; the rule stabilized in ruff 0.16 and ruff>=0.15.12 floats, so it landed on every branch at once.
  • Integration tests (all OSes) and Cloud tests — the broken spiceai/quickstart Spicepod archive from the registry.

I deliberately have not duplicated #176’s pyproject.toml/test.yml changes here — that would conflict with it and put a CI fix in a feature PR. Once #176 lands I will re-run these checks on this branch; expect them green with no further change.

…1/status entries

send_request_raw merged the session headers into the dict the caller passed
in, so a caller reusing one headers dict across calls silently accumulated
the session's headers. Build a merged copy instead; session headers still
take precedence, as in send_request.

runtime_status() checked that the response was a list but not that each
entry was an object, so a non-object entry surfaced as an AttributeError
from ConnectionDetails.from_dict rather than a SpiceAIError naming the
offending component.
@lukekim
lukekim enabled auto-merge (squash) July 29, 2026 20:21
auto-merge was automatically disabled July 29, 2026 22:14

Head branch was pushed to by a user without write access

@claudespice

Copy link
Copy Markdown
Author

Pushed 59160b9 — the second black failure on this branch, and like the first it was mine.

A test added later in the branch (test_does_not_mutate_caller_headers, added at tests/test_http.py:441) has a signature one character over black's 88-column default, so black wants it wrapped. Ruff here is configured for 120 columns, which is why it passed ruff and failed black. Arguments and behaviour are unchanged:

-    def test_does_not_mutate_caller_headers(self, mock_session_class: MagicMock) -> None:
+    def test_does_not_mutate_caller_headers(
+        self, mock_session_class: MagicMock
+    ) -> None:

Verified locally against the exact versions CI resolves (black==26.5.1, ruff==0.16.0, Python 3.14):

  • black --check spicepy tests24 files unchanged, clean
  • pytest tests/test_http.py tests/test_status.py66 passed

The three workflow runs for this push are sitting at action_required (runs 30495401088, 30495401010, 30495400990) — this PR is from a fork, so the runs need a maintainer to approve them before they execute. The checks will read grey rather than green until then; that is the approval gate, not a failure.

Still not mine, and still unchanged: Lint with ruff (PLR0917, reproduced on a pristine trunk with no PR applied) and Integration/Cloud tests (registry HTTP 503). Both are fixed by #176, which is green and has been waiting ~52h — I have flagged its blast radius over there rather than duplicating its changes here.

@claudespice

Copy link
Copy Markdown
Author

Status: this PR is APPROVED with zero checks reported — not because CI passed, but because it never ran. Both workflow runs for the current head 59160b9 sit at action_required since 2026-07-29T22:14Z (30495401088 Lint code base, 30495401010 Build package): fork PRs need a maintainer to approve the run. That approval is the only thing standing between this PR and a green board.

Local verification on 59160b9 (recorded again for convenience): black --check clean across 24 files, pytest 66 passed.

@claudespice

Copy link
Copy Markdown
Author

Following up on the action_required note above: the workflow runs were approved and CI has now reported. 63 checks, 58 green, 5 red — and none of the five is caused by this branch.

Lint with ruff (×4) — pre-existing on trunk. Two PLR0917 "Too many positional arguments" errors in spicepy/_client.py. A clean checkout of trunk (a199cfa) with ruff 0.16.1 reproduces both with nothing of this branch applied, so every open PR inherits them. Out of scope to fix here; #178 clears it against trunk, after which this job should go green on a re-run.

Cloud tests (Python 3.12) — fork secret. The job environment shows SPICE_API_KEY: empty; secrets are not exposed to pull requests from forks, so all 40 test_main.py cloud tests fail at connection rather than on anything they assert. Same on #177.

Everything this branch is actually responsible for is green: build on all four Python versions across Linux/macOS/Windows, unit, integration, pylint, mypy, black, bandit, and coverage. It remains APPROVED.

@claudespice

Copy link
Copy Markdown
Author

Correction to my previous comment: the ruff PLR0917 fix I pointed at is #176, not #178. I had missed that @lukekim already fixed this five days ago; #176 adds the same ignore entry and fixes the spice add spiceai/quickstart registry failure in test.yml. I have closed #178 as a duplicate.

#176 is green and awaiting review. Merging it turns the Lint with ruff job green here and on every other open PR in the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants