Only overwrite values when environment variables are set #62
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous implementation of `overwrite` would always overwrite values in the given struct, even if values existed. While this is the definition of `overwrite`, it unintentionally extended to `default` values as well. So even if a value was explicitly set on a struct, it would be overwritten with the "default" value set in envconfig. This was an unexpected behavior, since defaults should take the lowest precedence.
The new implementation has the following behavior with `overwrite`:
- If the struct field has the zero value and a default is set:
- If no environment variable is specified, the struct field will be populated with the default value.
- If an environment variable is specified, the struct field will be populate with the environment variable value.
- If the struct field has a non-zero value and a default is set:
- If no environment variable is specified, the struct field's existing value will be used (the default is ignored).
- If an environment variable is specified, the struct field's existing value will be overwritten with the environment variable value.
As part of this change, decoder interfaces are only processed when an environment (or a default) is present.
|
Interesting, thank you for the quick fix! I've confirmed this fixes the problem I was running into :) |
williamgcampbell
pushed a commit
to williamgcampbell/go-envconfig
that referenced
this pull request
Jul 26, 2022
The previous implementation of `overwrite` would always overwrite values in the given struct, even if values existed. While this is the definition of `overwrite`, it unintentionally extended to `default` values as well. So even if a value was explicitly set on a struct, it would be overwritten with the "default" value set in envconfig. This was an unexpected behavior, since defaults should take the lowest precedence.
The new implementation has the following behavior with `overwrite`:
- If the struct field has the zero value and a default is set:
- If no environment variable is specified, the struct field will be populated with the default value.
- If an environment variable is specified, the struct field will be populate with the environment variable value.
- If the struct field has a non-zero value and a default is set:
- If no environment variable is specified, the struct field's existing value will be used (the default is ignored).
- If an environment variable is specified, the struct field's existing value will be overwritten with the environment variable value.
As part of this change, decoder interfaces are only processed when an environment (or a default) is present.
sethvargo
added a commit
that referenced
this pull request
Jul 27, 2022
This reverts part of #62 to always process decoder interfaces for struct fields. This removes a breaking change where certain built-in types like net/url.URL have a custom unmarshaling function for the empty string. The other rules for "overwrite" still apply.
Merged
sethvargo
added a commit
that referenced
this pull request
Jul 27, 2022
This reverts part of #62 to always process decoder interfaces for struct fields. This removes a breaking change where certain built-in types like net/url.URL have a custom unmarshaling function for the empty string. The other rules for "overwrite" still apply.
sethvargo
added a commit
that referenced
this pull request
Dec 27, 2023
**:warning: Breaking!** Envconfig no longer runs decoders on unset values! To restore the old behavior, add the `decodeunset` struct field annotation or pass the `DefaultDecodeUnset` configuration option as `true`. Prior to this change, envconfig would always call decoding functions, even for unset or empty values. This proved problematic for some libraries like `url` and `zap` which implement `TextUnmarshaller`, but error on the empty string (#61). #62 changed the behavior to only call the decoder if a value was explicitly provided, but that broke users unexpectedly (#64), so the change was reverted. After much discussion, we decided to keep the existing behavior until the 1.0 release. However, after further reflection, I think we need to support a user-level configurable option. Some fields and structs may still want the decoder to run on empty values. This PR changes envconfig to only process a field if any of the following are true: - A value was provided (the value can be set to the empty string, there is a distinction between "unset" and "set to empty") - A default value was provided - The `decodeunset` struct field tag is set - The `DefaultDecodeUnset` configuration option is true
sethvargo
added a commit
that referenced
this pull request
Dec 27, 2023
**:warning: Breaking!** Envconfig no longer runs decoders on unset values! To restore the old behavior, add the `decodeunset` struct field annotation or pass the `DefaultDecodeUnset` configuration option as `true`. Prior to this change, envconfig would always call decoding functions, even for unset or empty values. This proved problematic for some libraries like `url` and `zap` which implement `TextUnmarshaller`, but error on the empty string (#61). #62 changed the behavior to only call the decoder if a value was explicitly provided, but that broke users unexpectedly (#64), so the change was reverted. After much discussion, we decided to keep the existing behavior until the 1.0 release. However, after further reflection, I think we need to support a user-level configurable option. Some fields and structs may still want the decoder to run on empty values. This PR changes envconfig to only process a field if any of the following are true: - A value was provided (the value can be set to the empty string, there is a distinction between "unset" and "set to empty") - A default value was provided - The `decodeunset` struct field tag is set - The `DefaultDecodeUnset` configuration option is true
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous implementation of
overwritewould always overwrite values in the given struct, even if values existed. While this is the definition ofoverwrite, it unintentionally extended todefaultvalues as well. So even if a value was explicitly set on a struct, it would be overwritten with the "default" value set in envconfig. This was an unexpected behavior, since defaults should take the lowest precedence.The new implementation has the following behavior with
overwrite:If the struct field has the zero value and a default is set:
If no environment variable is specified, the struct field will be populated with the default value.
If an environment variable is specified, the struct field will be populate with the environment variable value.
If the struct field has a non-zero value and a default is set:
If no environment variable is specified, the struct field's existing value will be used (the default is ignored).
If an environment variable is specified, the struct field's existing value will be overwritten with the environment variable value.
As part of this change, decoder interfaces are only processed when an environment (or a default) is present.
Closes #61
/cc @raserva @twmb