AIR CLI Integration: authenticate air get up front and fail fast#5729
Merged
Conversation
air get up front and fail fastair get up front and fail fast
Collaborator
Integration test reportCommit: 3eabc54
15 interesting tests: 6 FAIL, 4 SKIP, 3 RECOVERED, 1 KNOWN, 1 flaky
Top 23 slowest tests (at least 2 minutes):
|
2fb4758 to
9b8fd81
Compare
1cf6da6 to
827ff43
Compare
9b8fd81 to
387c976
Compare
maggiewang-db
approved these changes
Jul 6, 2026
| // mid-render instead of a clear "not authenticated" error here. | ||
| if _, err := w.CurrentUser.Me(ctx, iam.MeRequest{}); err != nil { | ||
| return authError(ctx, cmd, err) | ||
| } |
There was a problem hiding this comment.
Any failure from this Me() probe is routed through authError, which unconditionally emits UNAUTHENTICATED / PERMANENT / retryable=false (L82–87). So a transient failure reaching /me (429, 5xx, network blip) gets reported as a non-retryable auth error.
Consider only mapping to UNAUTHENTICATED when the error is actually authshaped (e.g. an apierr 401/403), and falling back to the transient classification otherwise.
| data.Sweep = buildSweepInfo(ctx, w, task) | ||
| } else if genAIComputeTask(run) == nil { | ||
| // The typed SDK drops ai_runtime_task, so read it from the raw run. | ||
| enrichFromRawRun(ctx, w, runID, &data) |
There was a problem hiding this comment.
w.Jobs.GetRun at L133 already fetched this run; fetchJobRun hits the same runs/get endpoint again. Can we reuse that response (or parse the raw payload once) to skip the second roundtrip?
The get command validated authentication only lazily: Config.Authenticate (in PreRunE) merely attaches credentials, so a bad PAT or missing profile surfaced as a confusing error partway through rendering, after a run's config had already been printed. Validate the workspace client up front instead: - PreRunE maps MustWorkspaceClient failures to an actionable auth error: a missing-profile hint when no default profile is set and --profile (-p) was not passed (ErrCannotConfigureDefault), otherwise "authentication was not successful". - RunE calls CurrentUser.Me before fetching or rendering anything, so a credential that resolves locally but is rejected by the workspace fails fast with the same clear message and no partial status/config output. Co-authored-by: Isaac
…_task runs The typed SDK's jobs.RunTask has no ai_runtime_task field, so w.Jobs.GetRun drops the task for runs submitted by the current `air run` (which emits ai_runtime_task). This left the Configuration box empty and Experiment / Accelerators as N/A for those runs. Mirror the Python CLI: for a single run with no gen_ai_compute_task, read the raw runs/get payload, take the ai_runtime_task command_path, and fill the config box from the sibling training_config.yaml plus the experiment and accelerator cells. Legacy gen_ai_compute_task runs keep their existing path. Co-authored-by: Isaac
Two fixes from review of the fail-fast/render commits: - authError only maps genuinely auth-shaped failures (missing profile, or an apierr 401/403 via ErrUnauthenticated/ErrPermissionDenied) to UNAUTHENTICATED/PERMANENT. A transient failure at the Me() probe (429, 5xx, network blip) is now reported as INTERNAL_ERROR/TRANSIENT so it stays retryable instead of a misleading non-retryable auth error. - `air get` fetched the run twice on the ai_runtime_task path: w.Jobs.GetRun (typed) then a second runs/get in enrichFromRawRun. Add fetchRun, which makes one runs/get call and unmarshals the body into both the typed jobs.Run (display path) and the raw jobRun (preserves ai_runtime_task). enrichFromRawRun now takes the already-fetched raw run — no second roundtrip. Tests: command-level not-found tests move to a real runs/get HTTP server (the run fetch is no longer the typed, mockable API); add fetchRun coverage and a transient-auth case. Co-authored-by: Isaac
387c976 to
3eabc54
Compare
yamlfmt normalizes the block scalar to `command: |-` (strip trailing newline); the CLI echoes the config back, so regenerate the acceptance golden to match. Fixes the `task fmt` CI gate. Co-authored-by: Isaac
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
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
Authenticate before any run status or config is fetched or printed:
MustWorkspaceClientfailure to an actionable auth error:--profile/-pnot passed (config.ErrCannotConfigureDefault) → "no default profile is set: pass --profile (-p) or configure a default profile in your .databrickscfg"CurrentUser.Mebefore fetching/rendering anything, so a credential that resolves locally but is rejected by the workspace also fails fast with the same clear message — and no partial status/config is shown.Both errors are permanent (not retryable) and, in
-o jsonmode, render as the standard error envelope (code: UNAUTHENTICATED).Why
air get JOB_RUN_IDvalidated authentication only lazily.MustWorkspaceClient(PreRunE) callsConfig.Authenticate, which merely attaches credentials (for a PAT it does no server-side check), so an invalid credential or missing profile surfaced as a confusing, generic failure partway through — after a run's config had already started rendering — instead of a clear, up-front error.Testing
TestGetRunAuthFailed— a rejectedCurrentUser.Meshort-circuits beforeGetRun(no run fetched, nothing rendered).TestAuthError— verifies the no-profile vs generic-auth message mapping (and that the cause is preserved).Mesuccess.go build ./..., the air unit tests, the air acceptance suite, and./task lint-q(0 issues) all pass.Based on
air-cli(post-#5685, so the command isair get JOB_RUN_ID).This pull request and its description were written by Isaac.