Skip to content

AIR CLI Integration: Logs (air logs command) #5970

Merged
riddhibhagwat-db merged 11 commits into
air-clifrom
air-logs-m4
Jul 24, 2026
Merged

AIR CLI Integration: Logs (air logs command) #5970
riddhibhagwat-db merged 11 commits into
air-clifrom
air-logs-m4

Conversation

@riddhibhagwat-db

@riddhibhagwat-db riddhibhagwat-db commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Changes

Implements the air logs JOB_RUN_ID command (previously a notImplemented stub) for the experimental AIR CLI. It fetches a run's training logs with a Bricklens-first, MLflow-fallback strategy:

  • Bricklens (primary): streams logs from the AiTraining log endpoint following an active run to completion, or tailing a completed one.
  • MLflow (fallback): when Bricklens is unavailable & gated off by a backend flag (FEATURE_DISABLED), not deployed (ENDPOINT_NOT_FOUND/404), or persistently failing. The command falls back to reading the run's MLflow log artifacts (chunk discovery + credential-vended download). The flag is evaluated server-side; the CLI only reads the response error code.

Flags:

  • --minutes restricts the fetch to the last N minutes (Bricklens time window).
  • --lines <N> is the tail the last N lines of a completed run. Mutually exclusive with --minutes.
  • --node, --retry is used to select a node / retry attempt. A past retry of a still-active run renders its (immutable) logs once instead of following the run.
  • --download-to / --review are rejected with a clear "not implemented" error (next PR follow up).

Why

air logs was the last unimplemented read command in the AIR CLI port. Bricklens is the primary log source, but it's behind a backend feature flag and isn't universally deployed, so the command must degrade gracefully to MLflow artifacts rather than fail. Bricklens is time-indexed (hence --minutes), while MLflow stores fixed log chunks (hence line-based --lines). the two flags map to what each backend can actually do.

Tests

  • Unit: classifyLogError fallback classification, --minutes/--lines window math, run-status projection, bounded dedup set, page draining + tail ordering, MLflow chunk listing/sorting and log-path discovery, flag validation, and an end-to-end Bricklens MLflow fallback through a mock server.
  • Acceptance: acceptance/experimental/air/logs/ (text + JSON streaming, --minutes, --lines, --retry, mutual-exclusion error, invalid ID, --download-to rejection) and logs-mlflow-fallback/ (Bricklens FEATURE_DISABLED, MLflow fallback, no-logs, text + JSON).
Screenshot 2026-07-24 at 4 14 21 PM

Implement the core of `air logs`: fetch a run's training logs from the
Bricklens-backed AiTrainingService endpoint, and on a backend-signaled failure
fall back to the MLflow log path. The feature flag is a backend SAFE flag — the
CLI never evaluates it; classifyLogError only reads the response error code
(FEATURE_DISABLED / ENDPOINT_NOT_FOUND / 404), and also falls back after
maxTransientFailures consecutive transient failures. The Bricklens/MLflow
decision is a single consolidated try/catch in fetchLogs.

Adds `--minutes N` (Bricklens-native time window) alongside `--lines N`
(MLflow-native line tail); the two are mutually exclusive and both flow through
the shared logRequest so either backend honors them. Streaming + status
resolution live in one file (logstream.go) as a small streamer type.

The MLflow fallback is a stub returning an explicit not-yet-implemented error
(phase 3 fills it in); --download-to and --review are rejected explicitly rather
than silently ignored.

Co-authored-by: Isaac
Add the `--retry` past-retry handling: a past retry of a still-active run has
immutable logs, so render them as a one-shot tail (drainStatic) instead of
following the run — otherwise the poll loop would wait forever for the run,
not the attempt, to finish. Mirrors handle_logs' viewing_past_retry. Exit code
still follows the run's result state (matching the Python `result_state ==
"SUCCESS"` rule), so a past retry of a running job prints its logs then exits
non-zero.

Also strengthen coverage of the phase-1 tail/streaming paths: a drainPages test
for boundary-second dedup and out-of-order skip, an acceptance case for --lines
tailing, and one for --retry.

Co-authored-by: Isaac
Replace the phase-1 stub with the real MLflow artifact path used when Bricklens
is gated off. It resolves the run's MLflow run id (via the existing mlflowIDs
helper), discovers the per-node log directory (logs/node_Y or the
attempt-prefixed logs/attempt_X/node_Y layout), lists the logs-N.chunk.txt chunk
files, and walks them newest-first — downloading each via the credential-vended
credentials-for-read pre-signed URL — until it has the requested tail, then
prints oldest-first. Mirrors the Python print_all_logs path.

--lines drives the tail as on the Bricklens path. MLflow chunk files are not
time-indexed, so --minutes can't restrict the window here; when only --minutes
is set the default tail cap applies and a debug line notes it was inapplicable.

Refactor the log-line and no-logs emitters into shared free functions
(emitLogLine / emitNoLogs) so the Bricklens and MLflow paths produce identical
raw and JSONL output instead of duplicating the logic.

Co-authored-by: Isaac
Make comments concise and self-contained: drop cross-references to the Python
CLI and remove restated-code noise. No behavior change.

Co-authored-by: Isaac
Add direct unit tests for the JSONL emit path (emitLogLine / emitNoLogs), the
requestPage transient-retry loop (retries then falls back, and retries then
succeeds), and displayState. Make retryCheckInterval a var so the retry-loop
tests run without the 3s production wait.

Co-authored-by: Isaac
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: f9326f4

Run: 30125032391

Env ❌​FAIL 🟨​KNOWN 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
❌​ aws-cli-is linux 8 1 3 4 320 1035 7:20
❌​ aws-cli-is windows 8 1 3 4 322 1033 15:35
❌​ azure-cli-is linux 8 1 3 4 320 1034 7:34
❌​ azure-cli-is windows 8 1 3 4 322 1032 18:21
❌​ gcp-cli-is linux 8 4 4 319 1036 11:03
❌​ gcp-cli-is windows 8 4 4 321 1034 18:23
16 interesting tests: 8 FAIL, 4 KNOWN, 4 SKIP
Test Name aws-cli-is linux aws-cli-is windows azure-cli-is linux azure-cli-is windows gcp-cli-is linux gcp-cli-is windows
🟨​ TestAccept 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K
❌​ TestAccept/bundle/deploy/mlops-stacks ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=direct ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=terraform ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/generate/auto-bind ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/generate/auto-bind/DATABRICKS_BUNDLE_ENGINE=terraform ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
❌​ TestAccept/bundle/resources/grants/volumes ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/resources/grants/volumes/DATABRICKS_BUNDLE_ENGINE=direct ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/resources/grants/volumes/DATABRICKS_BUNDLE_ENGINE=terraform ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 🟨​K 🟨​K
🟨​ TestFetchRepositoryInfoAPI_FromRepo/root 💚​R 💚​R 💚​R 💚​R 🟨​K 🟨​K
🟨​ TestFetchRepositoryInfoAPI_FromRepo/subdir 💚​R 💚​R 💚​R 💚​R 🟨​K 🟨​K

@riddhibhagwat-db
riddhibhagwat-db requested review from maggiewang-db, simonfaltum and vinchenzo-db and removed request for simonfaltum July 20, 2026 17:15
When Bricklens is unavailable, air logs falls back to a one-shot MLflow read.
For a run that is still active, that fallback found no artifacts yet and printed
"Run terminated in state RUNNING", which is contradictory. Branch emitNoLogs on
whether the run is terminal: a terminal run keeps its termination reason, while
an active run is reported as having no logs yet in its current state. The
reference Python implementation only emits the "terminated" message from its
terminal branch, so this matches that invariant. Extend TestEmitNoLogs with a
non-terminal case.

Co-authored-by: Isaac
return renderError(ctx, cmd, "INVALID_ARGS", "PERMANENT", false,
fmt.Errorf("invalid --minutes %d: must be positive", minutes))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

--node isn't validated for negatives like --lines/--minutes are.

Comment thread experimental/air/cmd/logmlflow.go Outdated
}
if mlflowRunID == "" || logDir == "" {
emitNoLogs(out, req, status)
return false, nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The behavior for mlflow path and bricklens path is different when no logs.
Brickelens: returns status.succeeded() -> exit 0
mlflow: return false, nil -> exit 1

Comment thread experimental/air/cmd/logstream.go Outdated
}
// A transient status blip should not abort a live stream.
log.Debugf(st.ctx, "air logs: failed to refresh run status: %v", err)
time.Sleep(retryCheckInterval)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On SIGINT, GetRun returns context.Canceled here, treated as a transient blip — so the loop sleeps+continues forever and Ctrl-C never exits.

…SIGINT

- Reject a negative --node, matching the --lines/--minutes validation.
- Align the MLflow fallback's no-logs exit code with the Bricklens path: both
  now return the run's outcome (SUCCESS -> 0) instead of MLflow returning a hard
  failure just because no logs exist.
- Stop the poll loop from spinning forever on Ctrl-C: a cancelled context is no
  longer treated as a transient status blip, and the inter-poll waits return
  promptly via sleepOrCancel.

Co-authored-by: Isaac
Restore parity with the Python CLI's efficiency/robustness:

- Build the Bricklens API client once per streamBricklensLogs call and reuse it
  across every page fetch in the poll loop, instead of rebuilding it on each
  request. Matches the Python client, which memoizes its workspace client.
- Give the MLflow pre-signed artifact download connect (10s) and
  response-header (60s) timeouts via a dedicated http.Client, so a stalled
  storage backend can't hang the command. Matches the Python download's
  (10, 60) timeouts; previously used http.DefaultClient with no timeout.

Co-authored-by: Isaac
Restore two more parities with the Python CLI:

- In -o json mode, a log line matching a fatal-failure pattern (OOM, NCCL
  timeout, CUDA error, segfault, etc.) now emits an ALERT event before its LOG
  event, giving an agent an immediate actionable signal. Previously only LOG
  events were emitted. Fatal patterns ported from the Python error_detection
  module.
- An explicit --lines 0 now prints no log lines (matching Python's --tail 0),
  distinguished from an unset --lines which still uses the default cap.

Co-authored-by: Isaac
While following an active run that hasn't emitted logs yet, print a spinner on
stderr ("Waiting for run to start (node N)...") so a provisioning run doesn't
look hung. Matches the Python CLI. The spinner is suppressed in --json mode and
auto-degrades to nothing on a non-interactive terminal, and is stopped before
the first log line (or the no-logs message) reaches stdout so frames don't smear.

Co-authored-by: Isaac
@riddhibhagwat-db
riddhibhagwat-db merged commit 861023c into air-cli Jul 24, 2026
21 of 22 checks passed
@riddhibhagwat-db
riddhibhagwat-db deleted the air-logs-m4 branch July 24, 2026 23:21
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.

3 participants