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.

Sign up for GitHub

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

Jump to bottom

Use dyn.InvalidValue to indicate absence #1507

Merged
merged 5 commits into from
Jun 19, 2024
Merged

Use dyn.InvalidValue to indicate absence #1507

merged 5 commits into from
Jun 19, 2024

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Jun 19, 2024

Changes

Previously, the functions Get and Index returned dyn.NilValue to indicate that a map key or sequence index wasn't found. This is a valid value, so we need to differentiate between actual absence and a real dyn.NilValue. We do this with the zero value of a dyn.Value (also captured in the constant dyn.InvalidValue).

Tests

  • Unit tests.
  • Renamed Get and Index to find and update all call sites.

@pietern pietern requested a review from shreyas-goenka June 19, 2024 13:48
@pietern
Copy link
Contributor Author

pietern commented Jun 19, 2024

Confirmed the integration tests are good as well.

@pietern pietern added this pull request to the merge queue Jun 19, 2024
Merged via the queue into main with commit b2c03ea Jun 19, 2024
5 checks passed
@pietern pietern deleted the dyn-nil branch June 19, 2024 15:30
@shreyas-goenka shreyas-goenka mentioned this pull request Jun 21, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 21, 2024
@shreyas-goenka
Return dyn.InvalidValue instead of dyn.NilValue when errors happen (
068c7cf 
#1514)

## Changes
With #1507 and
#1511 we are clarifying the
semantics associated with `dyn.InvalidValue` and `dyn.NilValue`. An
invalid value is the default zero value and is used to signals the
complete absence of the value.

A nil value, on the other hand, is a valid value for a piece of
configuration and signals explicitly setting a key to nil in the
configuration tree. In keeping with that theme, this PR returns
`dyn.InvalidValue` instead of `dyn.NilValue` at error sites. This change
is not expected to have a material change in behaviour and is being done
to set the right convention since we have well-defined semantics
associated with both `NilValue` and `InvalidValue`.

## Tests
Unit tests and integration tests pass. Also manually scanned the changes
and the associated call sites to verify the `NilValue` value itself was
not being relied upon.
pietern added a commit that referenced this pull request Jun 26, 2024
@pietern
Release v0.222.0
529a718 
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)).
@pietern pietern mentioned this pull request Jun 26, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
@pietern
Release v0.222.0 (#1529)
cdd6fe8 
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)).
@pietern pietern mentioned this pull request Jul 1, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 1, 2024
@pietern
Ignore dyn.NilValue when traversing value from dyn.Map (#1547)
da603c6 
## Changes

The map function ignores cases where either a key in a map is not
present or an index in a sequence is out of bounds. As of recently, we
retain nil values as valid values in a configuration tree. As such, it
makes sense to also ignore cases where a map or sequence is expected but
nil is found. This is semantically no different from an empty map where
a key is not found.

Without this fix, all calls to `dyn.Map` would need to be updated with
nil-checks at every path component.

Related PRs:
* #1507
* #1511

## Tests

Unit tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewnester andrewnester andrewnester approved these changes

@shreyas-goenka shreyas-goenka Awaiting requested review from shreyas-goenka

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pietern @andrewnester