Skip to content

feat: add health, readiness, and runtime status checks - #48

Merged
lukekim merged 4 commits into
trunkfrom
feat/health-ready-status
Jul 30, 2026
Merged

feat: add health, readiness, and runtime status checks#48
lukekim merged 4 commits into
trunkfrom
feat/health-ready-status

Conversation

@lukekim

@lukekim lukekim commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What

Adds three runtime checks to SpiceClient, wrapping /health, /v1/ready, and /v1/status:

// Liveness — unauthenticated.
if (!client.isHealthy()) {
    return;
}

// Readiness — a runtime can be healthy but still loading its datasets.
while (!client.isReady()) {
    Thread.sleep(1000);
}

// Per-component detail.
for (ConnectionDetails connection : client.runtimeStatus()) {
    System.out.printf("%s %s %s%n",
        connection.getName(), connection.getEndpoint(), connection.getStatus());
}

New public types: ConnectionDetails and ComponentStatus. Includes a runnable example at src/main/java/ai/spice/example/ExampleHealthAndStatus.java.

Why

None of these were reachable from Java without hand-rolling the HTTP calls. Java was the only SDK missing health and readiness with no in-flight work covering it — gospice and spice.js already have them, and equivalents are open for spicepy (#173) and spice-rs (#81).

/v1/status is missing from every SDK, and is strictly more informative than the boolean /v1/ready: it names which connection is not ready and where it is bound.

Three design choices worth flagging for review:

  • The probes return false instead of throwing when the runtime is unreachable, so they can be polled in a loop without a try/catch. runtimeStatus() does throw, since a caller asking for detail wants to know why it failed.
  • Naming follows the runtime_status / is_ready convention that spicepy #173 and spice-rs #81 settled on, adapted to Java (runtimeStatus(), isReady()), rather than gospice's older IsSpiceReady.
  • An unrecognized status maps to ComponentStatus.UNKNOWN rather than failing, so a newer runtime can add a status without breaking an older client. getRawStatus() returns it verbatim.

Also note: isReady() and runtimeStatus() send X-API-Key when an API key is configured, which Spice.ai Cloud requires. The existing refreshDataset does not — that looks like a separate bug and is left alone here.

Verification

  • mvn package -DskipTests (with -Dgpg.skip=true; local signing has no key)
  • mvn test — 231 of 232 pass, including 18 new tests in RuntimeStatusTest. They run against a JDK HttpServer on an ephemeral port, so no live runtime and no new test dependency is needed.
  • mvn spotbugs:check — clean
  • Verified end to end against a live spice run by running the new
    ExampleHealthAndStatus:
Spice runtime is healthy
Spice runtime is ready
Runtime status:
  http           127.0.0.1:8090           Ready
  flight         127.0.0.1:50051          Ready
  metrics        N/A                      Disabled
  opentelemetry  127.0.0.1:50051          Ready
  Note the `metrics` row — `N/A` / `Disabled` is exactly the case a boolean
  readiness check cannot express, and it parses correctly.

The single failure, ResetTest.testQueryAfterResetRebuildsClient ("Expected a connection/transport error, got: Please add arrow-compression module ... LZ4_FRAME"), reproduces on unmodified trunk and is unrelated to this change.

Adds isHealthy(), isReady(), and runtimeStatus(), wrapping /health,
/v1/ready, and /v1/status. None were reachable from Java without hand-rolling
the HTTP calls.

The probes return false rather than throwing when the runtime is unreachable,
so they can be polled directly in a loop — matching gospice and spice.js.

runtimeStatus() returns one ConnectionDetails per runtime connection, naming
which component is not ready and where it is bound. A ComponentStatus a newer
runtime introduces maps to UNKNOWN rather than failing, and getRawStatus()
keeps the verbatim value.

Naming follows the in-flight runtime_status/is_ready work in spicepy and
spice-rs, adapted to Java conventions.
Copilot AI review requested due to automatic review settings July 27, 2026 16:02

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 liveness/readiness probes and a detailed per-component status API to the Java SDK (SpiceClient) by wrapping the runtime’s /health, /v1/ready, and /v1/status endpoints, along with tests, docs, and an example.

Changes:

  • Add SpiceClient.isHealthy(), SpiceClient.isReady(), and SpiceClient.runtimeStatus() plus JSON parsing for /v1/status.
  • Introduce new public types ConnectionDetails and ComponentStatus to model runtime connection status.
  • Add a runnable example and README docs, plus a new RuntimeStatusTest suite.

Reviewed changes

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

Show a summary per file
File Description
src/main/java/ai/spice/SpiceClient.java Adds probe/status APIs and JSON parsing helpers for runtime health/readiness/status endpoints.
src/main/java/ai/spice/ConnectionDetails.java New public model type for one connection’s status, including raw status passthrough.
src/main/java/ai/spice/ComponentStatus.java New enum mapping wire status strings to stable SDK values with UNKNOWN fallback.
src/test/java/ai/spice/RuntimeStatusTest.java Adds unit tests for probes, request-building behavior, and status parsing.
src/main/java/ai/spice/example/ExampleHealthAndStatus.java Adds runnable example showing polling readiness and printing per-connection status.
README.md Documents the new health/readiness/status APIs and links to the example.

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

Comment thread src/main/java/ai/spice/SpiceClient.java Outdated
Comment thread src/main/java/ai/spice/SpiceClient.java
@lukekim lukekim self-assigned this Jul 27, 2026
@lukekim lukekim added the enhancement New feature or request label Jul 27, 2026
@lukekim lukekim added this to the v0.6.0 milestone Jul 27, 2026
@lukekim
lukekim requested review from Copilot and sgrebnov July 27, 2026 16:22

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

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/main/java/ai/spice/SpiceClient.java:1146

  • probe() lowercases the response and uses contains(expectedBody), which can produce false positives (e.g., a 200 body of "not ready" contains "ready"). For these probe endpoints a trimmed, case-insensitive equality check is safer and avoids locale-sensitive lowercasing.
            String body = response.body();
            return body != null && body.toLowerCase().contains(expectedBody);

src/main/java/ai/spice/SpiceClient.java:1171

  • buildProbeRequest() constructs the request URI by string-concatenating httpAddress and path. This can yield an unexpected double-slash path (or missing slash) when callers provide a base URI with/without a trailing /, leading to 404s against strict servers. Normalize the join to ensure exactly one / between base and path while preserving any base path segment.
        HttpRequest.Builder builder = HttpRequest.newBuilder()
                .uri(new URI(String.format("%s%s", httpAddress, path)))
                .header("X-Spice-User-Agent", Config.getUserAgent())
                .GET();

lukekim added 2 commits July 27, 2026 15:42
…gistry

`spice add spiceai/quickstart` makes every run depend on the Spicepod registry,
which is currently returning a body the CLI cannot unpack:

  Invalid argument: Failed to extract Spicepod archive: invalid Zip archive:
  Could not find EOCD

Declaring the same dataset inline removes the dependency. Covers all four sites
— three in build.yaml (including the PowerShell step) and one in publish.yaml.

Verified locally: taxi_trips loads and is queryable within seconds.

Mirrors spicepy#176.
Copilot AI review requested due to automatic review settings July 29, 2026 21:14

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.

🟡 Not ready to approve

There are a couple of correctness/robustness issues in the new HTTP probe implementation (locale-sensitive lowercasing and error-prone URI construction) that should be addressed before approval.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (3)

src/main/java/ai/spice/SpiceClient.java:1170

  • Building the request URI via string concatenation can produce double slashes (e.g., when httpAddress ends with /) and is generally more error-prone than using URI#resolve for joining a base URI with a path.
        HttpRequest.Builder builder = HttpRequest.newBuilder()
                .uri(new URI(String.format("%s%s", httpAddress, path)))
                .header("X-Spice-User-Agent", Config.getUserAgent())

src/main/java/ai/spice/SpiceClient.java:1146

  • String#toLowerCase() without an explicit locale is locale-dependent (e.g., Turkish locale edge cases) and can make the probe result vary by runtime environment; use Locale.ROOT for deterministic, locale-agnostic matching.
            String body = response.body();
            return body != null && body.toLowerCase().contains(expectedBody);

src/test/java/ai/spice/RuntimeStatusTest.java:215

  • The comment reads a bit garbled (“stack internal”); it looks like it meant “internal stack trace”.
            // The message should tell the user what to fix, not leak a stack internal.
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Two issues from review:

- probe() tested the body with contains(), so "not ready" — which contains
  "ready" — reported a still-loading runtime as ready. It now compares the
  trimmed body case-insensitively, since these endpoints return one token.
- buildProbeRequest() concatenated the address and path, producing
  "http://host:8090//v1/ready" for a base with a trailing slash. It now uses
  URI.resolve.

Adds tests for both, including "not ready" returned with a 200.
Copilot AI review requested due to automatic review settings July 29, 2026 23:08

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.

🟡 Not ready to approve

runtimeStatus()’s unreachable-runtime handling is too narrow (only direct ConnectException), which can yield less actionable error messages for common HttpClient I/O failures.

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

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (1)

src/main/java/ai/spice/SpiceClient.java:1234

  • runtimeStatus() only special-cases a direct ConnectException. In practice, HttpClient.send(...) failures for an unreachable runtime can surface as other IOException subtypes (or as an IOException whose cause is a ConnectException), which will fall into the generic catch (Exception) path and lose the clearer "runtime is unavailable at " message the method is trying to provide.
        } catch (Exception err) {
            logger.error("Runtime status failed: {}", err.getMessage());
            throw new ExecutionException("Failed to fetch runtime status due to error: " + err.toString(), err);
        }
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@lukekim
lukekim merged commit cf7bf6e into trunk Jul 30, 2026
20 checks passed
@lukekim
lukekim deleted the feat/health-ready-status branch July 30, 2026 17:09
lukekim added a commit that referenced this pull request Aug 1, 2026
…ardening (#51)

* docs: v0.7.0 release preparation — release notes, upgrade guide, QA hardening

- Release notes now cover the full v0.6.0..v0.7.0 delta: mTLS client
  certificates (#46) and health/readiness/status checks (#48) were never
  released and join the perf overhaul (#50); dependency table corrected
  against what v0.6.0 actually shipped (ADBC 0.22.0)
- New upgrade guide: docs/upgrade_guides/v0.6.0-to-v0.7.0.md (behavior
  changes, dependency-tree impacts, new capabilities)
- README installation snippets bumped to 0.7.0
- Integration-test hardening found during live-runtime QA:
  - testRefreshWithOptionsSpiceOSS was rerun-unsafe: its refresh_sql LIMIT
    persists in the accelerated table across runs and refreshes are async.
    Now self-heals the precondition, polls instead of fixed sleep, and
    restores the dataset afterward
  - availability probes use one retry instead of zero (resilient gating,
    still fast when no runtime is present)

QA: 284 tests green twice consecutively against a live spiced quickstart
(taxi_trips, 2.9M rows) plus mock-only runs; jar/sources/javadoc build clean.

* fix: address Copilot review — test resource management and comment accuracy

- FlightQueryTest: all four tests now close SpiceClient (and FlightStream)
  via try-with-resources
- testRefreshWithOptionsSpiceOSS: shrink/verify runs in try/finally with a
  guaranteed restore that waits for the async refresh to land; restore
  failures are logged, never masking the primary test failure
- TpchIntegrationTest: probe comment updated to match withMaxRetries(1)

Validated against a live spiced quickstart: FlightQueryTest green twice
consecutively with the dataset restored to full row count after each run;
full suite 284/284 + SpotBugs clean.
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