AIR CLI Integration: -h for yaml configuration - #6239
Merged
Conversation
riddhibhagwat-db
requested review from
ben-hansen-db,
maggiewang-db and
vinchenzo-db
and removed request for
ben-hansen-db
August 11, 2026 21:13
Collaborator
Integration test reportCommit: 512365f
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 6 slowest tests (at least 2 minutes):
|
ben-hansen-db
left a comment
ben-hansen-db
left a comment
Contributor
There was a problem hiding this comment.
Looks good just some nits and comments to fix before merging
|
|
||
| Fields: | ||
| num_accelerators Total number of GPUs to allocate. | ||
| accelerator_type Which accelerator to run on: GPU_1xA10, GPU_1xH100, or GPU_8xH100. |
Contributor
There was a problem hiding this comment.
Do we have a docs link where folks can see what GPU types are supported? It would be nice to link that here.
You can say for instance GPU_1xA10 ... see [list] for current listing of gpu's offered
Contributor
There was a problem hiding this comment.
similarly for accelerator_type below
| === polymorphic field reports both accepted shapes | ||
| >>> [CLI] experimental air run -h config.environment.dependencies | ||
| config.environment.dependencies | ||
| Either a path to a requirements.yaml file, or an inline list of packages. Not allowed alongside docker_image. |
Contributor
There was a problem hiding this comment.
not a path. Just an inline list of packages.
| // configFieldSummary is the one-line description used in a field listing: the | ||
| // first sentence of the help text, annotated when the field is required. | ||
| func configFieldSummary(f configField) string { | ||
| summary, _, found := strings.Cut(f.help, ". ") |
Contributor
There was a problem hiding this comment.
why is this cut on first period? maybe skip known abbreviations e.g. i.e. etc. when finding sentence boundary
`air run` takes a YAML config with ~35 fields across 9 nested structs, and
the only way to learn a field was to read runconfig.go or trip its validation
error. Add schema help addressed by dotted path:
air run -h config # list top-level fields
air run -h config.compute # list a section's fields
air run -h config.compute.accelerator_type
air run -h compute.accelerator_type # the config. prefix is optional
Descriptions live in `help:` struct tags beside the `yaml:` tags, so they sit
next to the validation rules that enforce them. Required-ness is a string
rather than a bool because validate() enforces it contextually:
docker_image.url is required only once docker_image is present.
The path must be a separate argument. cobra hardcodes a bool read of the help
flag in execute(), so a string -h is rejected outright and -h=<path> cannot
work; the Long text says so rather than leaving it to be discovered. Cobra
returns flag.ErrHelp before Args and required-flag validation, which is why
this needs no --file.
A reflection test fails if any schema field lacks a `help:` tag, so a new
field cannot merge undocumented — the same guard annotations.yml gets from
its PLACEHOLDER convention.
`--override` path validation (runconfig_override.go) and `-h config.<field>` (runconfig.go) each walked runConfig by reflection to resolve a dotted path. Collapse them onto one walk: describeStruct now records whether a field is a free-form map, and checkOverridePath resolves against the shared configSchema() tree instead of its own yamlFields/underlyingStruct/freeFormFields helpers, which are deleted. Behavior is unchanged. --override keeps its own error voice (the messages its tests pin), and a new test asserts both features resolve against the same schema so they can't drift apart again.
riddhibhagwat-db
force-pushed
the
air-config-help
branch
from
August 11, 2026 23:36
3f51cd4 to
512365f
Compare
riddhibhagwat-db
added a commit
that referenced
this pull request
Aug 13, 2026
## Summary **#6153 ("AIR CLI Migration: `--download-to` flag for logs") was lost from `air-cli`.** It merged on 2026-08-06, but `air-cli` was later rewound to `1fcb3c09a` before #6239 merged (08-12), and the rebuilt line (#6239 → #6244 → #6241) bypassed #6153. As a result `air-cli` today still carries the **pre-#6153 stub**: - `logs.go`: `--download-to is not implemented yet` (the flag is rejected) - no `logdownload.go` / `logdownload_test.go` - no `acceptance/experimental/air/logs-download/` test dir This PR restores #6153's change set onto the current `air-cli` tip. ## How Cherry-pick of #6153's original squash commit (`60cd876910cd`) onto `air-cli`. Verified equivalence to the original: - Every file except `logstream.go` is **byte-identical** to what #6153 landed. - `logstream.go` is re-merged against #6241's later edits to that file (git auto-merged it cleanly; both changes coexist). ## Testing - `go build ./experimental/air/...` — ok - `go test ./experimental/air/...` — 548 pass - `go test ./acceptance -run TestAccept/experimental/air` — 29 pass (incl. `logs` and the restored `logs-download`) - Package lints clean - Confirmed the `--download-to is not implemented` stub is gone and `logs.go` now wires the real implementation This pull request and its description were written 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
3 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
Adds databricks experimental air run -h config. — a help path that documents any field of the run YAML config from the command line.
-hconfig lists the top-level fields;-h config.compute listsa section's fields;-h config.compute.accelerator_typeshows one field's type, required-ness, and descriptionWhy
air runtakes a YAML config with 35 fields across 9 nested structs, and until now the only way to learn a field was to read runconfig.go or trip its validation error. This surfaces the schema in the CLI itself.Tests
Manual verification:
Informational error outputs for unknown fields:
