Skip to content

Commit

Permalink
Fixed generated YAML missing 'default' for empty values
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewnester committed Sep 10, 2024
1 parent b7ea53d commit e895614
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
14 changes: 14 additions & 0 deletions bundle/config/generate/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ func ConvertJobToValue(job *jobs.Job) (dyn.Value, error) {
value["tasks"] = dyn.NewValue(tasks, []dyn.Location{{Line: jobOrder.Get("tasks")}})
}

// We're processing job.Settings.Parameters separately to retain empty default values.
if len(job.Settings.Parameters) > 0 {
params := make([]dyn.Value, 0)
for _, parameter := range job.Settings.Parameters {
p := map[string]dyn.Value{
"name": dyn.NewValue(parameter.Name, []dyn.Location{}),
"default": dyn.NewValue(parameter.Default, []dyn.Location{}),
}
params = append(params, dyn.NewValue(p, []dyn.Location{}))
}

value["parameters"] = dyn.NewValue(params, []dyn.Location{{Line: jobOrder.Get("parameters")}})
}

return yamlsaver.ConvertToMapValue(job.Settings, jobOrder, []string{"format", "new_cluster", "existing_cluster_id"}, value)
}

Expand Down
9 changes: 9 additions & 0 deletions cmd/bundle/generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ func TestGenerateJobCommand(t *testing.T) {
},
},
},
Parameters: []jobs.JobParameterDefinition{
{
Name: "empty",
Default: "",
},
},
},
}, nil)

Expand Down Expand Up @@ -198,6 +204,9 @@ func TestGenerateJobCommand(t *testing.T) {
- task_key: notebook_task
notebook_task:
notebook_path: %s
parameters:
- name: empty
default: ""
`, filepath.Join("..", "src", "notebook.py")), string(data))

data, err = os.ReadFile(filepath.Join(srcDir, "notebook.py"))
Expand Down
2 changes: 2 additions & 0 deletions libs/dyn/yamlsaver/saver.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ func isScalarValueInString(v dyn.Value) bool {
switch v.MustString() {
case "true", "false":
return true
case "":
return true
default:
_, err := parseNumber(v.MustString())
return err == nil
Expand Down

0 comments on commit e895614

Please sign in to comment.