Uh oh!
There was an error while loading. Please reload this page.
Added support for complex variables - #1467
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
pietern
left a comment
There was a problem hiding this comment.
The dyn changes can use a bit more test coverage.
| type Variable struct { | ||
| // A type of the variable. This is used to validate the value of the variable | ||
| Type VariableType `json:"type,omitempty"` | ||
There was a problem hiding this comment.
Would we want to constrain the type of primitive types as well?
And otherwise, could we get away with not storing this at all?
There was a problem hiding this comment.
We indeed could get away with not having this but there are few benefits of having this
- It makes the intention of using complex variable explicit
- It allows for visual editing tools to easily identify that the variable is of complex type and disable editing
- It give the ability for future to extend the type with a known types, f.e.
job_clustersand etc. I can imagine restricting the primitive types can be beneficial as well
Uh oh!
There was an error while loading. Please reload this page.
| if v.Kind() == reflect.Interface { | ||
| options = append(options, includeZeroValuedScalars) | ||
| } | ||
There was a problem hiding this comment.
This is to capture a variable value equal to 0, right?
Previously this would have been done through the non-nil pointer and dereferencing that.
There was a problem hiding this comment.
It otherwise fails to assign dynamic complex value containing zero valued scalars
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
shreyas-goenka
commented
Jun 17, 2024
We'll also need a PR similar to #1398 to update the JSON schema generated. To allow complex variable references. |
Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| } | ||
| return variable, nil | ||
There was a problem hiding this comment.
Does the PR include test coverage for this, i.e. both ways of setting the variable, as well as defining a top level complex variable and then specifying a different type in the override?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| if v.Kind() == reflect.Interface { | ||
| options = append(options, includeZeroValuedScalars) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| // Skip resolution if there is a skip function and it returns true. | ||
| if m.skipFn != nil && m.skipFn(v) { | ||
| return dyn.InvalidValue, dynvar.ErrSkipResolution | ||
| } |
There was a problem hiding this comment.
This can be moved outside of dynvar.Resolve.
| } | ||
| return variable, nil | ||
pietern
left a comment
There was a problem hiding this comment.
Remaining can be addressed async.
stevenayers-bge
commented
Jun 26, 2024
Hi folks, great job on this PR. Will it allow you to access nested objects inside a variable, or will it just support array indexing? Example: bundle:
name: complex-variablesresources:
jobs:
my_job:
job_clusters:
- job_cluster_key: keynew_cluster: ${var.cluster}tasks:
- task_key: "task ${var.cluster.foo}"job_cluster_key: keyvariables:
cluster:
description: "A cluster definition"type: complexdefault:
foo: barspark_version: "13.2.x-scala2.11"node_type_id: "Standard_DS3_v2"num_workers: 2spark_conf:
spark.speculation: truespark.databricks.delta.retentionDurationCheck.enabled: false |
andrewnester
commented
Jun 26, 2024
yes, you can access complex variable fields |
Uh oh!
There was an error while loading. Please reload this page.
CLI: * Add link to documentation for Homebrew installation to README ([#1505](#1505)). * Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment variable if exists as config file ([#1325](#1325)). Bundles: The Terraform upgrade to v1.48.0 includes a fix for library order not being respected. * Fix conditional in query in `default-sql` template ([#1479](#1479)). * Remove user credentials specified in the Git origin URL ([#1494](#1494)). * Serialize dynamic value for `bundle validate` output ([#1499](#1499)). * Override variables with lookup value even if values has default value set ([#1504](#1504)). * Pause quality monitors when "mode: development" is used ([#1481](#1481)). * Return `fs.ModeDir` for Git folders in the workspace ([#1521](#1521)). * Upgrade TF provider to 1.48.0 ([#1527](#1527)). * Added support for complex variables ([#1467](#1467)). Internal: * Add randIntn function ([#1475](#1475)). * Avoid multiple file tree traversals on bundle deploy ([#1493](#1493)). * Clean up unused code ([#1502](#1502)). * Use `dyn.InvalidValue` to indicate absence ([#1507](#1507)). * Add ApplyPythonMutator ([#1430](#1430)). * Set bool pointer to disable lock ([#1516](#1516)). * Allow the any type to be set to nil in `convert.FromTyped` ([#1518](#1518)). * Properly deal with nil values in `convert.FromTyped` ([#1511](#1511)). * Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen ([#1514](#1514)). * PythonMutator: replace stdin/stdout with files ([#1512](#1512)). * Add context type and value to path rewriting ([#1525](#1525)). API Changes: * Added schedule CRUD commands to `databricks lakeview`. * Added subscription CRUD commands to `databricks lakeview`. * Added `databricks apps start` command. OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24) Dependency updates: * Bump golang.org/x/text from 0.15.0 to 0.16.0 ([#1482](#1482)). * Bump golang.org/x/term from 0.20.0 to 0.21.0 ([#1483](#1483)). * Bump golang.org/x/mod from 0.17.0 to 0.18.0 ([#1484](#1484)). * Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0 ([#1485](#1485)). * Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1 ([#1495](#1495)). * Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 ([#1496](#1496)). * Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0 ([#1522](#1522)).
CLI: * Add link to documentation for Homebrew installation to README ([#1505](#1505)). * Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment variable if exists as config file ([#1325](#1325)). Bundles: The Terraform upgrade to v1.48.0 includes a fix for library order not being respected. * Fix conditional in query in `default-sql` template ([#1479](#1479)). * Remove user credentials specified in the Git origin URL ([#1494](#1494)). * Serialize dynamic value for `bundle validate` output ([#1499](#1499)). * Override variables with lookup value even if values has default value set ([#1504](#1504)). * Pause quality monitors when "mode: development" is used ([#1481](#1481)). * Return `fs.ModeDir` for Git folders in the workspace ([#1521](#1521)). * Upgrade TF provider to 1.48.0 ([#1527](#1527)). * Added support for complex variables ([#1467](#1467)). Internal: * Add randIntn function ([#1475](#1475)). * Avoid multiple file tree traversals on bundle deploy ([#1493](#1493)). * Clean up unused code ([#1502](#1502)). * Use `dyn.InvalidValue` to indicate absence ([#1507](#1507)). * Add ApplyPythonMutator ([#1430](#1430)). * Set bool pointer to disable lock ([#1516](#1516)). * Allow the any type to be set to nil in `convert.FromTyped` ([#1518](#1518)). * Properly deal with nil values in `convert.FromTyped` ([#1511](#1511)). * Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen ([#1514](#1514)). * PythonMutator: replace stdin/stdout with files ([#1512](#1512)). * Add context type and value to path rewriting ([#1525](#1525)). API Changes: * Added schedule CRUD commands to `databricks lakeview`. * Added subscription CRUD commands to `databricks lakeview`. * Added `databricks apps start` command. OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24) Dependency updates: * Bump golang.org/x/text from 0.15.0 to 0.16.0 ([#1482](#1482)). * Bump golang.org/x/term from 0.20.0 to 0.21.0 ([#1483](#1483)). * Bump golang.org/x/mod from 0.17.0 to 0.18.0 ([#1484](#1484)). * Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0 ([#1485](#1485)). * Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1 ([#1495](#1495)). * Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 ([#1496](#1496)). * Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0 ([#1522](#1522)).
## Changes This PR: 1. Removes the custom added in https://github.com/databricks/cli/pull/1396/files for `variables.*.default`. It's no longer needed because with complex variables (#1467) `default` has a type of any. 2. Retains, and extends the override on `targets.*.variables.*`. Target override values can now be complex objects, not just primitive values. ## Tests Manually Before: Only primitive types were allowed. <img width="436" alt="Screenshot 2024-06-27 at 3 58 34 PM" src="https://github.com/databricks/cli/assets/88374338/de55be5c-9236-4ccc-b529-97ce7201b8e8"> After: An empty JSON schema is generated. All YAML values are acceptable. <img width="453" alt="Screenshot 2024-06-27 at 3 57 15 PM" src="https://github.com/databricks/cli/assets/88374338/7534b770-563f-4efc-b9c6-0af526dcc705">
## Changes
Added support for complex variables
Now it's possible to add and use complex variables as shown below
```
bundle:
name: complex-variables
resources:
jobs:
my_job:
job_clusters:
- job_cluster_key: key
new_cluster: ${var.cluster}
tasks:
- task_key: test
job_cluster_key: key
variables:
cluster:
description: "A cluster definition"
type: complex
default:
spark_version: "13.2.x-scala2.11"
node_type_id: "Standard_DS3_v2"
num_workers: 2
spark_conf:
spark.speculation: true
spark.databricks.delta.retentionDurationCheck.enabled: false
```
Fixes#1298
- [x] Support for complex variables
- [x] Allow variable overrides (with shortcut) in targets
- [x] Don't allow to provide complex variables via flag or env variable
- [x] Fail validation if complex value is used but not `type: complex`
provided
- [x] Support using variables inside complex variables ## Tests
Added unit tests
---------
Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>CLI: * Add link to documentation for Homebrew installation to README ([#1505](#1505)). * Fix `databricks configure` to use `DATABRICKS_CONFIG_FILE` environment variable if exists as config file ([#1325](#1325)). Bundles: The Terraform upgrade to v1.48.0 includes a fix for library order not being respected. * Fix conditional in query in `default-sql` template ([#1479](#1479)). * Remove user credentials specified in the Git origin URL ([#1494](#1494)). * Serialize dynamic value for `bundle validate` output ([#1499](#1499)). * Override variables with lookup value even if values has default value set ([#1504](#1504)). * Pause quality monitors when "mode: development" is used ([#1481](#1481)). * Return `fs.ModeDir` for Git folders in the workspace ([#1521](#1521)). * Upgrade TF provider to 1.48.0 ([#1527](#1527)). * Added support for complex variables ([#1467](#1467)). Internal: * Add randIntn function ([#1475](#1475)). * Avoid multiple file tree traversals on bundle deploy ([#1493](#1493)). * Clean up unused code ([#1502](#1502)). * Use `dyn.InvalidValue` to indicate absence ([#1507](#1507)). * Add ApplyPythonMutator ([#1430](#1430)). * Set bool pointer to disable lock ([#1516](#1516)). * Allow the any type to be set to nil in `convert.FromTyped` ([#1518](#1518)). * Properly deal with nil values in `convert.FromTyped` ([#1511](#1511)). * Return `dyn.InvalidValue` instead of `dyn.NilValue` when errors happen ([#1514](#1514)). * PythonMutator: replace stdin/stdout with files ([#1512](#1512)). * Add context type and value to path rewriting ([#1525](#1525)). API Changes: * Added schedule CRUD commands to `databricks lakeview`. * Added subscription CRUD commands to `databricks lakeview`. * Added `databricks apps start` command. OpenAPI commit 7437dabb9dadee402c1fc060df4c1ce8cc5369f0 (2024-06-24) Dependency updates: * Bump golang.org/x/text from 0.15.0 to 0.16.0 ([#1482](#1482)). * Bump golang.org/x/term from 0.20.0 to 0.21.0 ([#1483](#1483)). * Bump golang.org/x/mod from 0.17.0 to 0.18.0 ([#1484](#1484)). * Bump golang.org/x/oauth2 from 0.20.0 to 0.21.0 ([#1485](#1485)). * Bump github.com/briandowns/spinner from 1.23.0 to 1.23.1 ([#1495](#1495)). * Bump github.com/spf13/cobra from 1.8.0 to 1.8.1 ([#1496](#1496)). * Bump github.com/databricks/databricks-sdk-go from 0.42.0 to 0.43.0 ([#1522](#1522)).
## Changes This PR: 1. Removes the custom added in https://github.com/databricks/cli/pull/1396/files for `variables.*.default`. It's no longer needed because with complex variables (#1467) `default` has a type of any. 2. Retains, and extends the override on `targets.*.variables.*`. Target override values can now be complex objects, not just primitive values. ## Tests Manually Before: Only primitive types were allowed. <img width="436" alt="Screenshot 2024-06-27 at 3 58 34 PM" src="https://github.com/databricks/cli/assets/88374338/de55be5c-9236-4ccc-b529-97ce7201b8e8"> After: An empty JSON schema is generated. All YAML values are acceptable. <img width="453" alt="Screenshot 2024-06-27 at 3 57 15 PM" src="https://github.com/databricks/cli/assets/88374338/7534b770-563f-4efc-b9c6-0af526dcc705">
Changes
Added support for complex variables
Now it's possible to add and use complex variables as shown below
Fixes#1298
type: complexprovidedTests
Added unit tests