flatten defaults to ensure env var load; expand flat settings to ensure override precedence [Fixes #71] [Fixes #160] #164
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
…ngs to ensure override precedence in merge
|
|
| @@ -908,8 +910,11 @@ func (v *Viper) InConfig(key string) bool { | |||
| func SetDefault(key string, value interface{}) { v.SetDefault(key, value) } | |||
| func (v *Viper) SetDefault(key string, value interface{}) { | |||
| // If alias passed in, then set the proper default | |||
| key = v.realKey(strings.ToLower(key)) | |||
| v.defaults[key] = value | |||
| flat := bellows.FlattenPrefixed(value, key) | |||
There was a problem hiding this comment.
We flatten the value if it is Struct or Map. Therefore, if we have a Config defined as a struct and define defaults as an instance of the Config struct, then we can set any or all defaults in a single call.
harnash
added a commit
to harnash/watcher
that referenced
this pull request
May 17, 2016
This code comes from spf13/viper#164 which by the time of this commit wasn't merged yet.
|
This seems like an important improvement, I'll try to take a look soon. In the meantime, if you could rebase it, it would be appreciated. |
|
I think this PR has a different approach to what was merged in #195 . It flattens everything, while we now create maps for everything. I wonder if correct behaviour would have been possible with your approach, because I think it would not have the same performance implications, though I doubt it. |
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.
There are some issues with nested settings precedence, due to some settings arriving from a flat map (flags, env vars) and some settings arriving from a nested map (existing structs, config files).
Right now
AutomaticEnvonly notices env vars if the flat path has been declared as a key. The best place to ensure a key is declared is in the defaults. Therefore we create a flatmap when setting the defaults.However, if any flat keys are unexpanded when
Unmarshaling to a Struct or Map, the flat keys will be ignored in favor of the existing expanded structure. Therefore we expand all keys before unmarshaling to the final config.This should solve all of the issues in #160 but we probably need a test for nested config and precedence, as proposed in #163.
Also related: #145, since with this change
Unmarshalsuccessfully merges config from env vars, pflags, defaults, and existing structures. I used this change to implement Viper config in ebay/fabio.