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

Return dyn.InvalidValue instead of dyn.NilValue when errors happen #1514

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Return dyn.InvalidValue instead of dyn.NilValue when errors happen #1514

merged 2 commits into from
Jun 21, 2024

Conversation

shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Jun 20, 2024 •
edited
Loading

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.

@shreyas-goenka
Copy link
Contributor Author

shreyas-goenka commented Jun 20, 2024 •
edited
Loading

gopatch file used:

@@
@@
-return dyn.NilValue, err
+return dyn.InvalidValue, err

@@
@@
-return dyn.NilValue, fmt.Errorf(...)
+return dyn.InvalidValue, fmt.Errorf(...)

@@
@@
-return dyn.NilValue, errorf(...)
+return dyn.InvalidValue, errorf(...)

@shreyas-goenka shreyas-goenka changed the title Use dyn.InvalidValue consistently when returning errors Return dyn.InvalidValue instead of dyn.NilValue when errors happen Jun 21, 2024
@pietern pietern marked this pull request as ready for review June 21, 2024 13:46
@shreyas-goenka shreyas-goenka added this pull request to the merge queue Jun 21, 2024
Merged via the queue into main with commit 068c7cf Jun 21, 2024
5 checks passed
@shreyas-goenka shreyas-goenka deleted the use-invalid-for-errors branch June 21, 2024 14:28
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)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pietern pietern pietern approved these changes

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
@shreyas-goenka @pietern