Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed variable override in target with full variable syntax #1749

Merged
merged 5 commits into from
Sep 4, 2024

Conversation

andrewnester
Copy link
Contributor

Changes

This PR makes sure that both of this override syntax for variables work correctly

targets:
  dev:
    variables:
      cluster1:
        spark_version: "14.2.x-scala2.11"
        node_type_id: "Standard_DS3_v2"
        num_workers: 4
        spark_conf:
          spark.speculation: false
          spark.databricks.delta.retentionDurationCheck.enabled: false
      cluster2:
        default:
          spark_version: "14.2.x-scala2.11"
          node_type_id: "Standard_DS3_v2"
          num_workers: 4
          spark_conf:
            spark.speculation: false
            spark.databricks.delta.retentionDurationCheck.enabled: false

Tests

Added regression test

bundle/config/root.go Outdated Show resolved Hide resolved
bundle/config/root.go Outdated Show resolved Hide resolved
return dyn.NewValue(map[string]dyn.Value{
"type": typeV,
"default": variable,
}, variable.Locations()), nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only way to override for complex variables?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, we support both shorthand and full one. Ambiguity can be if someone defined a complex variable and overrides it with

default: some value goes here

Then we can't really understand if it's shorthand or full syntax

andrewnester and others added 4 commits September 4, 2024 16:53
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
@andrewnester andrewnester added this pull request to the merge queue Sep 4, 2024
Merged via the queue into main with commit 7203084 Sep 4, 2024
5 checks passed
@andrewnester andrewnester deleted the fix/allow-default-value-override branch September 4, 2024 17:24
andrewnester added a commit that referenced this pull request Sep 5, 2024
CLI:
 * Do not error if we cannot prompt for a profile in `auth login` ([#1745](#1745)).

Bundles:
 * Pass along $AZURE_CONFIG_FILE to Terraform process ([#1734](#1734)).
 * Add prompt when a pipeline recreation happens ([#1672](#1672)).
 * Use materialized views in the default-sql template ([#1709](#1709)).
 * Update templates to latest LTS DBR ([#1715](#1715)).
 * Make lock optional in the JSON schema ([#1738](#1738)).
 * Do not suppress normalisation diagnostics for resolving variables ([#1740](#1740)).
 * Include a permissions section in all templates ([#1713](#1713)).
 * Fixed complex variables are not being correctly merged from include files ([#1746](#1746)).
 * Fixed variable override in target with full variable syntax ([#1749](#1749)).

Internal:
 * Consider serverless clusters as compatible for Python wheel tasks ([#1733](#1733)).
 * PythonMutator: explain missing package error ([#1736](#1736)).
 * Add `dyn.Time` to box a timestamp with its original string value ([#1732](#1732)).
 * Fix streaming of stdout, stdin, stderr in cobra test runner ([#1742](#1742)).

Dependency updates:
 * Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0 ([#1741](#1741)).
github-merge-queue bot pushed a commit that referenced this pull request Sep 5, 2024
CLI:
* Do not error if we cannot prompt for a profile in `auth login`
([#1745](#1745)).

Bundles:

As of this release CLI will show a prompt is if there are configuration
changes which will lead to a DLT recreation.
Users can skip the prompt by specifying the `--auto-approve` flag

* Pass along $AZURE_CONFIG_FILE to Terraform process
([#1734](#1734)).
* Add prompt when a pipeline recreation happens
([#1672](#1672)).
* Use materialized views in the default-sql template
([#1709](#1709)).
* Update templates to latest LTS DBR
([#1715](#1715)).
* Make lock optional in the JSON schema
([#1738](#1738)).
* Do not suppress normalisation diagnostics for resolving variables
([#1740](#1740)).
* Include a permissions section in all templates
([#1713](#1713)).
* Fixed complex variables are not being correctly merged from include
files ([#1746](#1746)).
* Fixed variable override in target with full variable syntax
([#1749](#1749)).

Internal:
* Consider serverless clusters as compatible for Python wheel tasks
([#1733](#1733)).
* PythonMutator: explain missing package error
([#1736](#1736)).
* Add `dyn.Time` to box a timestamp with its original string value
([#1732](#1732)).
* Fix streaming of stdout, stdin, stderr in cobra test runner
([#1742](#1742)).

Dependency updates:
* Bump github.com/Masterminds/semver/v3 from 3.2.1 to 3.3.0
([#1741](#1741)).

---------

Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants