Skip to content

Commit f711157

Browse files
committed
[air] Deliver env vars only via the common Jobs env-var API (no sidecar)
Match the Python CLI (#2237609): env vars and secrets ride the job-level environment_variables profile the converter emits, not the legacy env_vars.json / secret_env_vars.json workspace side-channel. Drop that sidecar staging from buildArtifacts and delete the now-dead entry encoders and filename constants. The other launch artifacts buildArtifacts stages (command.sh, training_config.yaml, requirements.yaml, hyperparameters.yaml) are unchanged — those fixed the "requirements.yaml not found" harness error and are unrelated to env vars. Known gap (verified on staging): the vendored databricks-sdk-go jobs.JobSettings/Task has no environment_variables field, so the DABs bundle schema strips it client-side ("unknown field") before the wire. Env vars therefore aren't delivered on the DABs path until the SDK carries the common Jobs env-var API. The Jobs server already accepts the field (gated by allowEnvironmentVariables); this is purely a DABs/SDK-schema lag. Co-authored-by: Isaac
1 parent e05d89e commit f711157

4 files changed

Lines changed: 64 additions & 103 deletions

File tree

experimental/air/cmd/rundabs.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,23 +107,28 @@ func renderBundle(cfg *runConfig, configPath string) (string, error) {
107107
return string(body) + bundleTargetsBlock(), nil
108108
}
109109

110-
// writeBundleProject renders databricks.yml (via renderBundle) plus command.sh and,
111-
// for a code_source snapshot, the user's code tree, into a temp bundle root. Deploy
112-
// uploads the whole root as an immutable-folder snapshot. Returns the root and a
113-
// cleanup func.
110+
// writeBundleProject renders databricks.yml plus the launch artifacts the AI Runtime
111+
// harness reads (command.sh, training_config.yaml, requirements.yaml, and — when
112+
// present — hyperparameters.yaml and env-var sidecars) and, for a code_source
113+
// snapshot, the user's code tree, into a temp bundle root. Deploy uploads the whole
114+
// root as an immutable-folder snapshot. Returns the root and a cleanup func.
114115
func writeBundleProject(ctx context.Context, cfg *runConfig, configPath string) (string, func(), error) {
115116
body, err := renderBundle(cfg, configPath)
116117
if err != nil {
117118
return "", func() {}, err
118119
}
120+
artifacts, err := buildArtifacts(cfg, configPath)
121+
if err != nil {
122+
return "", func() {}, err
123+
}
119124
bundleRoot, err := os.MkdirTemp("", "air-dabs-*")
120125
if err != nil {
121126
return "", func() {}, err
122127
}
123128
cleanup := func() { _ = os.RemoveAll(bundleRoot) }
124129

125-
// Copy the code_source working tree into the bundle root first, so a stray
126-
// command.sh / databricks.yml in the user's tree can't shadow ours below.
130+
// Copy the code_source working tree first, so a stray command.sh / databricks.yml
131+
// in the user's tree can't shadow the generated files written below.
127132
if cfg.CodeSource != nil && cfg.CodeSource.Snapshot != nil {
128133
if err := stageCodeSource(ctx, cfg.CodeSource.Snapshot, configPath, bundleRoot); err != nil {
129134
cleanup()
@@ -136,9 +141,10 @@ func writeBundleProject(ctx context.Context, cfg *runConfig, configPath string)
136141
return "", func() {}, err
137142
}
138143

139-
// command.sh: the entrypoint the task's command_path points at.
140-
if cfg.Command != nil {
141-
if err := os.WriteFile(filepath.Join(bundleRoot, bundleCommandScript), []byte(*cfg.Command), 0o600); err != nil {
144+
// The launch artifacts the harness expects co-located with command.sh, the same
145+
// set the retired ephemeral path uploaded (buildArtifacts).
146+
for _, it := range artifacts {
147+
if err := os.WriteFile(filepath.Join(bundleRoot, it.name), it.data, 0o600); err != nil {
142148
cleanup()
143149
return "", func() {}, err
144150
}

experimental/air/cmd/rundabs_test.go

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,11 @@ import (
1515
)
1616

1717
func TestWriteBundleProject(t *testing.T) {
18-
cfg := &runConfig{
19-
ExperimentName: "exp",
20-
Command: new("echo hi"),
21-
Compute: &computeConfig{AcceleratorType: "GPU_1xA10", NumAccelerators: 1},
22-
}
18+
configPath := writeConfigFile(t, "train.yaml", minimalConfig)
19+
cfg, err := loadRunConfig(configPath)
20+
require.NoError(t, err)
2321

24-
root, cleanup, err := writeBundleProject(t.Context(), cfg, "train.yaml")
22+
root, cleanup, err := writeBundleProject(t.Context(), cfg, configPath)
2523
require.NoError(t, err)
2624
defer cleanup()
2725

@@ -32,10 +30,12 @@ func TestWriteBundleProject(t *testing.T) {
3230
assert.Contains(t, string(body), "immutable_folder: true")
3331
assert.Contains(t, string(body), "mode: development")
3432

35-
// command.sh holds the run's command verbatim.
33+
// The launch artifacts the AI Runtime harness reads are staged next to command.sh.
34+
assert.FileExists(t, filepath.Join(root, bundleCommandScript))
35+
assert.FileExists(t, filepath.Join(root, trainingConfigName))
3636
script, err := os.ReadFile(filepath.Join(root, bundleCommandScript))
3737
require.NoError(t, err)
38-
assert.Equal(t, "echo hi", string(script))
38+
assert.Equal(t, "python train.py", string(script))
3939

4040
// cleanup removes the temp root.
4141
cleanup()
@@ -49,14 +49,16 @@ func TestWriteBundleProjectStagesCodeSource(t *testing.T) {
4949
require.NoError(t, os.MkdirAll(filepath.Join(src, "pkg"), 0o755))
5050
require.NoError(t, os.WriteFile(filepath.Join(src, "pkg", "mod.py"), []byte("x=1"), 0o644))
5151

52-
cfg := &runConfig{
53-
ExperimentName: "exp",
54-
Command: new("python train.py"),
55-
Compute: &computeConfig{AcceleratorType: "GPU_1xA10", NumAccelerators: 1},
56-
CodeSource: &codeSourceConfig{Type: "snapshot", Snapshot: &snapshotSourceConfig{RootPath: src}},
57-
}
52+
configPath := writeConfigFile(t, "train.yaml", minimalConfig+`
53+
code_source:
54+
type: snapshot
55+
snapshot:
56+
root_path: `+src+`
57+
`)
58+
cfg, err := loadRunConfig(configPath)
59+
require.NoError(t, err)
5860

59-
root, cleanup, err := writeBundleProject(t.Context(), cfg, "train.yaml")
61+
root, cleanup, err := writeBundleProject(t.Context(), cfg, configPath)
6062
require.NoError(t, err)
6163
defer cleanup()
6264

@@ -69,35 +71,42 @@ func TestWriteBundleProjectStagesCodeSource(t *testing.T) {
6971

7072
func TestWriteBundleProjectCommandShadowProtection(t *testing.T) {
7173
// A command.sh in the user's tree must not shadow the one we generate from the
72-
// run's command: our write happens after the tree copy.
74+
// run's command: the artifact writes happen after the tree copy.
7375
src := t.TempDir()
7476
require.NoError(t, os.WriteFile(filepath.Join(src, bundleCommandScript), []byte("STALE"), 0o644))
7577

76-
cfg := &runConfig{
77-
ExperimentName: "exp",
78-
Command: new("FRESH"),
79-
Compute: &computeConfig{AcceleratorType: "GPU_1xA10", NumAccelerators: 1},
80-
CodeSource: &codeSourceConfig{Type: "snapshot", Snapshot: &snapshotSourceConfig{RootPath: src}},
81-
}
78+
configPath := writeConfigFile(t, "train.yaml", minimalConfig+`
79+
code_source:
80+
type: snapshot
81+
snapshot:
82+
root_path: `+src+`
83+
`)
84+
cfg, err := loadRunConfig(configPath)
85+
require.NoError(t, err)
8286

83-
root, cleanup, err := writeBundleProject(t.Context(), cfg, "train.yaml")
87+
root, cleanup, err := writeBundleProject(t.Context(), cfg, configPath)
8488
require.NoError(t, err)
8589
defer cleanup()
8690

91+
// minimalConfig's command is "python train.py", not the STALE tree copy.
8792
script, err := os.ReadFile(filepath.Join(root, bundleCommandScript))
8893
require.NoError(t, err)
89-
assert.Equal(t, "FRESH", string(script))
94+
assert.Equal(t, "python train.py", string(script))
9095
}
9196

9297
func TestWriteBundleProjectRejectsUnconvertible(t *testing.T) {
9398
// A gate failure (here: a $CODE_SOURCE_PATH command) surfaces before any temp
9499
// directory is created.
95-
cfg := &runConfig{
96-
ExperimentName: "exp",
97-
Command: new("cd $CODE_SOURCE_PATH && python train.py"),
98-
Compute: &computeConfig{AcceleratorType: "GPU_1xA10", NumAccelerators: 1},
99-
}
100-
_, _, err := writeBundleProject(t.Context(), cfg, "train.yaml")
100+
configPath := writeConfigFile(t, "train.yaml", `
101+
experiment_name: exp
102+
command: cd $CODE_SOURCE_PATH && python train.py
103+
compute:
104+
accelerator_type: GPU_1xA10
105+
num_accelerators: 1
106+
`)
107+
cfg, err := loadRunConfig(configPath)
108+
require.NoError(t, err)
109+
_, _, err = writeBundleProject(t.Context(), cfg, configPath)
101110
require.Error(t, err)
102111
}
103112

experimental/air/cmd/runupload.go

Lines changed: 4 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@ package aircmd
33
import (
44
"bytes"
55
"context"
6-
"encoding/json"
76
"fmt"
87
"io"
9-
"maps"
108
"os"
119
"path/filepath"
12-
"slices"
13-
"strings"
1410

1511
"github.com/databricks/cli/libs/filer"
1612
"go.yaml.in/yaml/v3"
@@ -24,8 +20,6 @@ const (
2420
commandScriptName = "command.sh"
2521
requirementsName = "requirements.yaml"
2622
hyperparametersName = "hyperparameters.yaml"
27-
envVarsName = "env_vars.json"
28-
secretEnvVarsName = "secret_env_vars.json"
2923
)
3024

3125
// maxConfigYAMLBytes caps training_config.yaml. It is referenced by the Jobs
@@ -102,59 +96,14 @@ func buildArtifacts(cfg *runConfig, configPath string) ([]uploadItem, error) {
10296
items = append(items, uploadItem{hyperparametersName, data})
10397
}
10498

105-
// The ai_runtime_task proto carries no inline env vars or secrets; stage them
106-
// as JSON files co-located with command.sh for the server-side launcher.
107-
if len(cfg.EnvVariables) > 0 {
108-
data, err := json.Marshal(envVarEntries(cfg.EnvVariables))
109-
if err != nil {
110-
return nil, fmt.Errorf("failed to serialize env_variables: %w", err)
111-
}
112-
items = append(items, uploadItem{envVarsName, data})
113-
}
114-
if len(cfg.Secrets) > 0 {
115-
data, err := json.Marshal(secretEnvVarEntries(cfg.Secrets))
116-
if err != nil {
117-
return nil, fmt.Errorf("failed to serialize secrets: %w", err)
118-
}
119-
items = append(items, uploadItem{secretEnvVarsName, data})
120-
}
99+
// Env vars and secrets are NOT staged as sidecar files: they ride the common
100+
// Jobs env-var API (the environment_variables profile the converter emits, see
101+
// exportbundle.go), matching the Python CLI which dropped the env_vars.json /
102+
// secret_env_vars.json side-channel.
121103

122104
return items, nil
123105
}
124106

125-
// envVarEntry is one entry in env_vars.json.
126-
type envVarEntry struct {
127-
Name string `json:"name"`
128-
Value string `json:"value"`
129-
}
130-
131-
// secretEnvVarEntry is one entry in secret_env_vars.json. The YAML side is
132-
// {ENV_VAR: "scope/key"}; the launcher wants the split form.
133-
type secretEnvVarEntry struct {
134-
Name string `json:"name"`
135-
SecretScope string `json:"secret_scope"`
136-
SecretKey string `json:"secret_key"`
137-
}
138-
139-
// envVarEntries renders env_variables sorted by name for deterministic output.
140-
func envVarEntries(vars map[string]string) []envVarEntry {
141-
out := make([]envVarEntry, 0, len(vars))
142-
for _, name := range slices.Sorted(maps.Keys(vars)) {
143-
out = append(out, envVarEntry{Name: name, Value: vars[name]})
144-
}
145-
return out
146-
}
147-
148-
// secretEnvVarEntries renders secrets sorted by name for deterministic output.
149-
func secretEnvVarEntries(secrets map[string]string) []secretEnvVarEntry {
150-
out := make([]secretEnvVarEntry, 0, len(secrets))
151-
for _, name := range slices.Sorted(maps.Keys(secrets)) {
152-
scope, key, _ := strings.Cut(secrets[name], "/")
153-
out = append(out, secretEnvVarEntry{Name: name, SecretScope: scope, SecretKey: key})
154-
}
155-
return out
156-
}
157-
158107
// uploadArtifacts writes each artifact into the launch directory, overwriting and
159108
// creating parents as needed.
160109
//

experimental/air/cmd/runupload_test.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ func TestBuildArtifacts_InlineRequirementsAndParameters(t *testing.T) {
8383
assert.Contains(t, req, "- torch")
8484
}
8585

86-
func TestBuildArtifacts_EnvVarsAndSecrets(t *testing.T) {
86+
func TestBuildArtifacts_EnvVarsNotStagedAsSidecars(t *testing.T) {
87+
// Env vars and secrets ride the common Jobs env-var API (the converter's
88+
// environment_variables profile), not sidecar files — matching the Python CLI,
89+
// which dropped the env_vars.json / secret_env_vars.json side-channel.
8790
path := writeConfigFile(t, "run.yaml", "x: y\n")
8891
cfg := &runConfig{
8992
Command: new("echo hi"),
@@ -93,14 +96,8 @@ func TestBuildArtifacts_EnvVarsAndSecrets(t *testing.T) {
9396

9497
items, err := buildArtifacts(cfg, path)
9598
require.NoError(t, err)
96-
assert.Subset(t, itemNames(items), []string{envVarsName, secretEnvVarsName})
97-
98-
byName := map[string][]byte{}
99-
for _, it := range items {
100-
byName[it.name] = it.data
101-
}
102-
assert.JSONEq(t, `[{"name":"WANDB","value":"demo"}]`, string(byName[envVarsName]))
103-
assert.JSONEq(t, `[{"name":"HF_TOKEN","secret_scope":"myscope","secret_key":"hf"}]`, string(byName[secretEnvVarsName]))
99+
assert.NotContains(t, itemNames(items), "env_vars.json")
100+
assert.NotContains(t, itemNames(items), "secret_env_vars.json")
104101
}
105102

106103
func TestBuildArtifacts_RequirementsFile(t *testing.T) {

0 commit comments

Comments
 (0)