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

Ignore dyn.NilValue when traversing value from dyn.Map #1547

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Jul 1, 2024

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:

Tests

Unit tests pass.

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 a 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.
@pietern pietern requested a review from shreyas-goenka July 1, 2024 12:23
pietern added a commit that referenced this pull request Jul 1, 2024
Issue #1545 describes how a nil entry in the sync block caused an error.

The fix for this issue is in #1547. This change adds end-to-end test coverage.
@pietern pietern added this pull request to the merge queue Jul 1, 2024
Merged via the queue into main with commit da603c6 Jul 1, 2024
5 checks passed
@pietern pietern deleted the dyn-map-on-nil branch July 1, 2024 13:07
github-merge-queue bot pushed a commit that referenced this pull request Jul 1, 2024
## Changes

Issue #1545 describes how a nil entry in the sync block caused an error.

The fix for this issue is in #1547. This change adds end-to-end test
coverage.

## Tests

New test passes on top of #1547.
pietern added a commit that referenced this pull request Jul 3, 2024
Bundles:

As of this release you can interact with bundles when running the CLI on DBR (e.g. via the Web Terminal).

 * Fix non-default project names not working in dbt-sql template ([#1500](#1500)).
 * Improve `bundle validate` output ([#1532](#1532)).
 * Fixed resolving variable references inside slice variable ([#1550](#1550)).
 * Fixed bundle not loading when empty variable is defined ([#1552](#1552)).
 * Use `vfs.Path` for filesystem interaction ([#1554](#1554)).
 * Replace `vfs.Path` with extension-aware filer when running on DBR ([#1556](#1556)).

Internal:
 * merge.Override: Fix handling of dyn.NilValue ([#1530](#1530)).
 * Compare `.Kind()` instead of direct equality checks on a `dyn.Value` ([#1520](#1520)).
 * PythonMutator: register product in user agent extra ([#1533](#1533)).
 * Ignore `dyn.NilValue` when traversing value from `dyn.Map` ([#1547](#1547)).
 * Add extra tests for the sync block ([#1548](#1548)).
 * PythonMutator: add diagnostics ([#1531](#1531)).
 * PythonMutator: support omitempty in PyDABs ([#1513](#1513)).
 * PythonMutator: allow insert 'resources' and 'resources.jobs' ([#1555](#1555)).
@pietern pietern mentioned this pull request Jul 3, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jul 3, 2024
Bundles:

As of this release you can interact with bundles when running the CLI on
DBR (e.g. via the Web Terminal).

* Fix non-default project names not working in dbt-sql template
([#1500](#1500)).
* Improve `bundle validate` output
([#1532](#1532)).
* Fixed resolving variable references inside slice variable
([#1550](#1550)).
* Fixed bundle not loading when empty variable is defined
([#1552](#1552)).
* Use `vfs.Path` for filesystem interaction
([#1554](#1554)).
* Replace `vfs.Path` with extension-aware filer when running on DBR
([#1556](#1556)).

Internal:
* merge.Override: Fix handling of dyn.NilValue
([#1530](#1530)).
* Compare `.Kind()` instead of direct equality checks on a `dyn.Value`
([#1520](#1520)).
* PythonMutator: register product in user agent extra
([#1533](#1533)).
* Ignore `dyn.NilValue` when traversing value from `dyn.Map`
([#1547](#1547)).
* Add extra tests for the sync block
([#1548](#1548)).
* PythonMutator: add diagnostics
([#1531](#1531)).
* PythonMutator: support omitempty in PyDABs
([#1513](#1513)).
* PythonMutator: allow insert 'resources' and 'resources.jobs'
([#1555](#1555)).
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