AIR CLI Integration: Logs (air logs command) #5970
Open
riddhibhagwat-db wants to merge 5 commits into
Open
Conversation
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
Contributor
Approval status: pending
|
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
Collaborator
Integration test reportCommit: a594c75
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 13 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
Flags:
--minutesrestricts 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,--retryis 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/--revieware rejected with a clear "not implemented" error (next PR follow up).Why
air logswas 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