Skip to content

Commit

Permalink
Allow unknown properties in the config file for template initializati…
Browse files Browse the repository at this point in the history
…on (#1315)

## Changes
Before we would error if a property was defined in the config file, that
was not defined in the schema.

## Tests
Unit tests. Also manually that the e2e flow works file.

Before:
```
shreyas.goenka@THW32HFW6T playground % cli bundle init default-python --config-file config.json

Welcome to the default Python template for Databricks Asset Bundles!
Error: failed to load config from file config.json: property include_pytho is not defined in the schema
```

After:
```
shreyas.goenka@THW32HFW6T playground % cli bundle init default-python --config-file config.json

Welcome to the default Python template for Databricks Asset Bundles!
Workspace to use (auto-detected, edit in 'test/databricks.yml'): https://dbc-a39a1eb1-ef95.cloud.databricks.com

✨ Your new project has been created in the 'test' directory!

Please refer to the README.md file for "getting started" instructions.
See also the documentation at https://docs.databricks.com/dev-tools/bundles/index.html.
```
  • Loading branch information
shreyas-goenka authored Mar 26, 2024
1 parent e3717ba commit b503804
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
13 changes: 12 additions & 1 deletion libs/template/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,26 @@ func validateSchema(schema *jsonschema.Schema) error {

// Reads json file at path and assigns values from the file
func (c *config) assignValuesFromFile(path string) error {
// Load the config file.
// It's valid to set additional properties in the config file that are not
// defined in the schema. They will be filtered below. Thus for the duration of
// the LoadInstance call, we disable the additional properties check,
// to allow those properties to be loaded.
c.schema.AdditionalProperties = true
configFromFile, err := c.schema.LoadInstance(path)
c.schema.AdditionalProperties = false

if err != nil {
return fmt.Errorf("failed to load config from file %s: %w", path, err)
}

// Write configs from the file to the input map, not overwriting any existing
// configurations.
for name, val := range configFromFile {
// If a property is not defined in the schema, skip it.
if _, ok := c.schema.Properties[name]; !ok {
continue
}
// If a value is already assigned, keep the original value.
if _, ok := c.values[name]; ok {
continue
}
Expand Down
11 changes: 11 additions & 0 deletions libs/template/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *te
assert.Equal(t, "this-is-not-overwritten", c.values["string_val"])
}

func TestTemplateConfigAssignValuesFromFileFiltersPropertiesNotInTheSchema(t *testing.T) {
c := testConfig(t)

err := c.assignValuesFromFile("./testdata/config-assign-from-file-unknown-property/config.json")
assert.NoError(t, err)

// assert only the known property is loaded
assert.Len(t, c.values, 1)
assert.Equal(t, "i am a known property", c.values["string_val"])
}

func TestTemplateConfigAssignDefaultValues(t *testing.T) {
c := testConfig(t)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"unknown_prop": 123
"unknown_prop": 123,
"string_val": "i am a known property"
}

0 comments on commit b503804

Please sign in to comment.