Skip to content

Commit 7987c78

Browse files
authored
air: flatten code_source configuration (#6927)
## Changes - Allow snapshot fields directly under `code_source` without requiring `type` or a nested `snapshot` block. - Preserve legacy configurations and overrides while normalizing supported forms to one internal path. - Hide legacy fields from help and reject mixed forms with actionable errors. ## Why The snapshot discriminator and wrapper are redundant now that snapshot is the only supported code source type. Flattening the configuration simplifies workload YAML and keeps future snapshot fields on one implementation path. ## Tests - `./task checks` - `go test ./cmd/air` - `go test ./acceptance -run TestAccept/air/config-help -tail` - `go vet ./cmd/air` - Built the PR CLI and successfully ran the AIR hello-world and a one-GPU Ray Core cookbook example with flattened `code_source`, including snapshot upload and extraction. - `./task test` ran 11,580 tests; affected AIR tests passed, but an unrelated `libs/dyn/jsonloader` assertion fails on changed Go JSON error text. - Full lint is blocked by a golangci-lint/Staticcheck `buildir` panic in dependency package `poll`. _This PR was written by Codex._ --------- Co-authored-by: Caroline Chen <324939130+caroline-db@users.noreply.github.com>
1 parent 13c4be6 commit 7987c78

10 files changed

Lines changed: 183 additions & 52 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* Support snapshot fields directly under `code_source` without requiring `type` or a nested `snapshot` block. ([#6927](https://github.com/databricks/cli/pull/6927))

‎acceptance/air/config-help/output.txt‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,29 +115,34 @@ config.compute.accelerator_type
115115
Type: string
116116
Required: no
117117

118-
=== conditionally required leaf
119-
>>> [CLI] air run -h config.code_source.type
120-
config.code_source.type
121-
Kind of code source. Must be "snapshot", the only supported type.
118+
=== code source fields
119+
>>> [CLI] air run -h config.code_source
120+
config.code_source
121+
Local code to upload and make available to the run.
122122

123-
Type: string
124-
Required: when code_source is set
123+
Fields:
124+
root_path Root of the code source to archive.
125+
remote_volume Volume to upload the archive to.
126+
git Pin the snapshot to a specific git revision.
127+
include_paths Restrict the archive to these paths, relative to root_path and without "..".
125128

126-
=== union field reports both accepted shapes
127-
>>> [CLI] air run -h config.code_source.snapshot.git.remote
128-
config.code_source.snapshot.git.remote
129+
Use "-h config.code_source.<field>" for details on a field.
130+
131+
=== union field
132+
>>> [CLI] air run -h config.code_source.git.remote
133+
config.code_source.git.remote
129134
No longer supported: the snapshot archives your local copy. Only false is accepted; use commit to pin a revision.
130135

131136
Type: bool or string
132137
Required: no
133138

134-
=== git-pinned subdirectory snapshots document root_path scoping
135-
>>> [CLI] air run -h config.code_source.snapshot.root_path
136-
config.code_source.snapshot.root_path
139+
=== git-pinned subdirectory documents root_path scoping
140+
>>> [CLI] air run -h config.code_source.root_path
141+
config.code_source.root_path
137142
Root of the code source to archive. A git-pinned subdirectory packages only that subtree.
138143

139144
Type: string
140-
Required: when code_source.snapshot is set
145+
Required: when code_source is set
141146

142147
=== the config. prefix is optional
143148
>>> [CLI] air run -h compute.num_accelerators

‎acceptance/air/config-help/script‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ trace $CLI air run -h config.environment.version
2222
title "leaf field"
2323
trace $CLI air run -h config.compute.accelerator_type
2424

25-
title "conditionally required leaf"
26-
trace $CLI air run -h config.code_source.type
25+
title "code source fields"
26+
trace $CLI air run -h config.code_source
2727

28-
title "union field reports both accepted shapes"
29-
trace $CLI air run -h config.code_source.snapshot.git.remote
28+
title "union field"
29+
trace $CLI air run -h config.code_source.git.remote
3030

31-
title "git-pinned subdirectory snapshots document root_path scoping"
32-
trace $CLI air run -h config.code_source.snapshot.root_path
31+
title "git-pinned subdirectory documents root_path scoping"
32+
trace $CLI air run -h config.code_source.root_path
3333

3434
title "the config. prefix is optional"
3535
trace $CLI air run -h compute.num_accelerators

‎cmd/air/convert_to_dabs.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func convertToDabs(ctx context.Context, cfg *runConfig, configPath, bundleDir st
130130
// artifact location is set bundle-wide via workspace.artifact_path, not
131131
// per-code-source, so a per-source Volume isn't representable here.
132132
if snap.RemoteVolume != nil {
133-
return nil, nil, errors.New("code_source.snapshot.remote_volume is not supported by convert-to-dabs; set workspace.artifact_path in the bundle instead")
133+
return nil, nil, errors.New("remote_volume in code_source is not supported by convert-to-dabs; set workspace.artifact_path in the bundle instead")
134134
}
135135
}
136136

‎cmd/air/convert_to_dabs_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ code_source:
374374
loaded, err := loadRunConfig(path)
375375
require.NoError(t, err)
376376
_, _, err = convertToDabs(t.Context(), loaded, path, filepath.Dir(path))
377-
require.ErrorContains(t, err, "remote_volume is not supported")
377+
require.ErrorContains(t, err, "remote_volume in code_source is not supported")
378378
}
379379

380380
// env_variables / secrets / parameters ride as sidecar files (the ai_runtime_task

‎cmd/air/runconfig.go‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -421,52 +421,72 @@ func (s *stringOrInt) UnmarshalYAML(node *yaml.Node) error {
421421

422422
// codeSourceConfig is the `code_source` block. Only the "snapshot" type exists.
423423
type codeSourceConfig struct {
424-
Type string `yaml:"type" help:"Kind of code source. Must be \"snapshot\", the only supported type." required:"when code_source is set"`
425-
Snapshot *snapshotSourceConfig `yaml:"snapshot" help:"Which local directory to archive and upload." required:"when code_source.type is \"snapshot\""`
424+
Type *string `yaml:"type,omitempty" hidden:"yes"`
425+
Snapshot *snapshotSourceConfig `yaml:",inline"`
426+
LegacySnapshot *snapshotSourceConfig `yaml:"snapshot,omitempty" hidden:"yes"`
426427
}
427428

428429
func (c *codeSourceConfig) validate() error {
429-
if c.Type != "snapshot" {
430-
return fmt.Errorf("code_source.type must be 'snapshot', got %q", c.Type)
430+
if c.Type != nil && strings.TrimSpace(*c.Type) == "" {
431+
return errors.New("code_source.type cannot be empty; omit it to use snapshot")
431432
}
432-
if c.Snapshot == nil {
433-
return errors.New("code_source.type='snapshot' requires a snapshot configuration")
433+
if c.Type != nil && *c.Type != "snapshot" {
434+
return fmt.Errorf("code_source.type must be 'snapshot', got %q", *c.Type)
434435
}
435-
return c.Snapshot.validate()
436+
if c.Snapshot != nil && c.LegacySnapshot != nil {
437+
return errors.New("code_source cannot combine the nested 'snapshot' block with direct fields; remove the 'snapshot' wrapper and place all fields directly under code_source")
438+
}
439+
440+
snapshot := c.Snapshot
441+
prefix := "code_source"
442+
if snapshot == nil {
443+
snapshot = c.LegacySnapshot
444+
prefix = "code_source.snapshot"
445+
}
446+
if snapshot == nil {
447+
return errors.New("code_source requires snapshot configuration with root_path")
448+
}
449+
if err := snapshot.validate(prefix); err != nil {
450+
return err
451+
}
452+
453+
c.Snapshot = snapshot
454+
c.LegacySnapshot = nil
455+
return nil
436456
}
437457

438458
// snapshotSourceConfig describes a local directory to tar and upload.
439459
type snapshotSourceConfig struct {
440-
RootPath string `yaml:"root_path" help:"Root of the code source to archive. A git-pinned subdirectory packages only that subtree." required:"when code_source.snapshot is set"`
460+
RootPath string `yaml:"root_path" help:"Root of the code source to archive. A git-pinned subdirectory packages only that subtree." required:"when code_source is set"`
441461
RemoteVolume *string `yaml:"remote_volume" help:"Volume to upload the archive to. Must start with /Volumes/."`
442462
Git *gitRef `yaml:"git" help:"Pin the snapshot to a specific git revision."`
443463
IncludePaths []string `yaml:"include_paths" help:"Restrict the archive to these paths, relative to root_path and without \"..\". Omit to include everything."`
444464
}
445465

446-
func (s *snapshotSourceConfig) validate() error {
466+
func (s *snapshotSourceConfig) validate(prefix string) error {
447467
if strings.TrimSpace(s.RootPath) == "" {
448-
return errors.New("code_source.snapshot.root_path cannot be empty")
468+
return fmt.Errorf("%s.root_path cannot be empty", prefix)
449469
}
450470

451471
if s.RemoteVolume != nil && !strings.HasPrefix(*s.RemoteVolume, "/Volumes/") {
452-
return errors.New("code_source.snapshot.remote_volume must start with '/Volumes/'")
472+
return fmt.Errorf("%s.remote_volume must start with '/Volumes/'", prefix)
453473
}
454474

455475
// A non-nil but empty include_paths is an explicit mistake (omit it instead).
456476
if s.IncludePaths != nil && len(s.IncludePaths) == 0 {
457-
return errors.New("code_source.snapshot.include_paths cannot be an empty list; either omit it or provide paths")
477+
return fmt.Errorf("%s.include_paths cannot be an empty list; either omit it or provide paths", prefix)
458478
}
459479
for _, p := range s.IncludePaths {
460480
p = strings.TrimSpace(p)
461481
if p == "" {
462-
return errors.New("code_source.snapshot.include_paths entry cannot be empty")
482+
return fmt.Errorf("%s.include_paths entry cannot be empty", prefix)
463483
}
464484
if strings.HasPrefix(p, "/") {
465-
return fmt.Errorf("code_source.snapshot.include_paths must be relative paths, got: %s", p)
485+
return fmt.Errorf("%s.include_paths must be relative paths, got: %s", prefix, p)
466486
}
467487
// No parent traversal: snapshots must stay within root_path.
468488
if slices.Contains(strings.Split(p, "/"), "..") {
469-
return fmt.Errorf("code_source.snapshot.include_paths cannot contain '..' traversal, got: %s", p)
489+
return fmt.Errorf("%s.include_paths cannot contain '..' traversal, got: %s", prefix, p)
470490
}
471491
}
472492

@@ -616,6 +636,7 @@ type configField struct {
616636
typeName string
617637
required string
618638
help string
639+
hidden bool
619640
// freeForm marks a user-keyed map (parameters/secrets/env_variables): no
620641
// children, yet any sub-path into it is valid.
621642
freeForm bool
@@ -651,7 +672,7 @@ func resolveConfigField(path string) (configField, error) {
651672
return configField{}, fmt.Errorf("%q is not an object, so it has no field %q", current.path, part)
652673
}
653674
child, ok := findConfigChild(current.children, part)
654-
if !ok {
675+
if !ok || child.hidden {
655676
return configField{}, unknownConfigFieldError(current, part, strings.Split(trimmed, ".")[:i+1])
656677
}
657678
current = child
@@ -692,6 +713,9 @@ func cutLast(s, sep string) (before, after string, found bool) {
692713
func unknownConfigFieldError(parent configField, part string, matched []string) error {
693714
names := make([]string, 0, len(parent.children))
694715
for _, c := range parent.children {
716+
if c.hidden {
717+
continue
718+
}
695719
names = append(names, configLeafName(c.path))
696720
}
697721
slices.Sort(names)
@@ -749,7 +773,13 @@ func describeStruct(t reflect.Type, prefix string) []configField {
749773
if tag == "" || tag == "-" {
750774
continue
751775
}
752-
name, _, _ := strings.Cut(tag, ",")
776+
name, options, _ := strings.Cut(tag, ",")
777+
if slices.Contains(strings.Split(options, ","), "inline") {
778+
if nested := underlyingConfigStruct(f.Type); nested != nil {
779+
out = append(out, describeStruct(nested, prefix)...)
780+
}
781+
continue
782+
}
753783
if name == "" || name == "-" {
754784
continue
755785
}
@@ -759,6 +789,7 @@ func describeStruct(t reflect.Type, prefix string) []configField {
759789
typeName: configTypeName(f.Type),
760790
required: f.Tag.Get("required"),
761791
help: f.Tag.Get("help"),
792+
hidden: f.Tag.Get("hidden") == "yes",
762793
freeForm: freeFormConfigFields[name],
763794
}
764795
if nested := underlyingConfigStruct(f.Type); nested != nil && !field.freeForm {
@@ -833,10 +864,16 @@ func renderConfigField(w io.Writer, f configField) {
833864

834865
width := 0
835866
for _, c := range f.children {
867+
if c.hidden {
868+
continue
869+
}
836870
width = max(width, len(configLeafName(c.path)))
837871
}
838872
fmt.Fprintf(w, "\n Fields:\n")
839873
for _, c := range f.children {
874+
if c.hidden {
875+
continue
876+
}
840877
fmt.Fprintf(w, " %-*s %s\n", width, configLeafName(c.path), configFieldSummary(c))
841878
}
842879
fmt.Fprintf(w, "\nUse \"-h %s.<field>\" for details on a field.\n", f.path)

‎cmd/air/runconfig_load.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,15 @@ func finishRunConfigLoad(path string, document *yaml.Node, validationYAML []byte
102102
if err != nil {
103103
return nil, err
104104
}
105+
// YAML decodes omitted and null pointers alike. Preserve explicit nulls so validation rejects them.
106+
if cfg.CodeSource != nil {
107+
if cfg.CodeSource.Type == nil && codeSourceFieldPresent(document, "type") {
108+
cfg.CodeSource.Type = new(string)
109+
}
110+
if cfg.CodeSource.LegacySnapshot == nil && codeSourceFieldPresent(document, "snapshot") {
111+
cfg.CodeSource.LegacySnapshot = &snapshotSourceConfig{}
112+
}
113+
}
105114
if err := validateRunConfig(cfg); err != nil {
106115
return nil, err
107116
}
@@ -115,6 +124,15 @@ func finishRunConfigLoad(path string, document *yaml.Node, validationYAML []byte
115124
return cfg, nil
116125
}
117126

127+
func codeSourceFieldPresent(document *yaml.Node, field string) bool {
128+
codeSource, _ := mappingValue(document.Content[0], "code_source")
129+
if codeSource == nil || codeSource.Kind != yaml.MappingNode {
130+
return false
131+
}
132+
fieldNode, _ := mappingValue(codeSource, field)
133+
return fieldNode != nil
134+
}
135+
118136
func stripYAMLComments(node *yaml.Node) {
119137
node.HeadComment = ""
120138
node.LineComment = ""

‎cmd/air/runconfig_override.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ func checkOverridePath(parts []string, node configField, fullPath string) error
6767
if !ok {
6868
names := make([]string, 0, len(node.children))
6969
for _, c := range node.children {
70+
if c.hidden {
71+
continue
72+
}
7073
names = append(names, configLeafName(c.path))
7174
}
7275
slices.Sort(names)

‎cmd/air/runconfig_override_test.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,15 @@ func TestLoadRunConfigWithOverrides(t *testing.T) {
138138
assert.Equal(t, "0", cfg.EnvVariables["RANK"])
139139
})
140140

141-
t.Run("intermediate maps are auto-created", func(t *testing.T) {
142-
cfg, err := loadRunConfigWithOverrides(t.Context(), writeConfig(t, overrideBaseConfig), []string{"code_source.type=snapshot", "code_source.snapshot.root_path=/Workspace/x"})
143-
require.NoError(t, err)
144-
require.NotNil(t, cfg.CodeSource)
145-
require.NotNil(t, cfg.CodeSource.Snapshot)
146-
assert.Equal(t, "/Workspace/x", cfg.CodeSource.Snapshot.RootPath)
147-
})
141+
for _, path := range []string{"code_source.root_path", "code_source.snapshot.root_path"} {
142+
t.Run(path+" override", func(t *testing.T) {
143+
cfg, err := loadRunConfigWithOverrides(t.Context(), writeConfig(t, overrideBaseConfig), []string{path + "=/Workspace/x"})
144+
require.NoError(t, err)
145+
require.NotNil(t, cfg.CodeSource)
146+
require.NotNil(t, cfg.CodeSource.Snapshot)
147+
assert.Equal(t, "/Workspace/x", cfg.CodeSource.Snapshot.RootPath)
148+
})
149+
}
148150

149151
t.Run("unity catalog image override applies", func(t *testing.T) {
150152
cfg, err := loadRunConfigWithOverrides(t.Context(), writeConfig(t, overrideBaseConfig), []string{"environment.unity_catalog_image=main.air.training:prod"})

0 commit comments

Comments
 (0)