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

AllKeys returns keys in reverse sorted order #1836

Closed
wants to merge 62 commits into from

Conversation

dustmop
Copy link

@dustmop dustmop commented May 20, 2024

Returning keys in a stable manner prevents non-determinism. A bug happening in allSettings manifests under certain orders of keys, by using reverse order this bug is never triggered.

hush-hush and others added 30 commits February 12, 2019 14:20
This allows users to set configuration that would have precedence on env
variable. This is very useful when some post-processing is done on the
configuration.
Adding a method to merge config in the override section
Fix AllSettings when some map keys contain the key delimiter
Merge changes from original repo
Try using `UnmarshalStrict`, if it fails, print the errors as a warning and try again with plain `Unmarshal`.

Errors look this way:
```
2019/03/26 15:11:33 warning reading config file: yaml: unmarshal errors:
  line 6: key "api_key" already set in map
```
…rebased

Warn when a yaml file has duplicated keys
This can be used to enforce a config schema
By creating a new Viper for each test instead of using the global one
Enable storing a set of known valid config keys
define custom install to make go modules work
djmitche and others added 27 commits July 20, 2021 11:00
Copy CI workflows from `spf13/viper` repo
Setting up codescanning for viper
Before, we would set entries to nil to unset them
Make AllKeys and AllSettings return set-but-empty keys
Add Unset method
We want the ability to pull the entire configuration without the default
to know what a user explicitly configured
…efault

Add a AllSettingsWithoutDefault method
Since this repo is owned by the whole team, any changes should ping all members
of this team for reviews.
return permissions errors when opening config files
Open Source Fork Monitor allowlist
Fix Viper 'UnmarshalKey' behavior to include all sources

Viper 'UnmarshalKey' seems to have been designed to work on leaf
settings only. When being called on a group of settings, it would not
merge all sources (default, env vars, config, override, ...).

This means that using the `Set()` method could change the returned value
from 'UnmarshalKey' in unexpected ways (i.e. would no longer return the
value from the configuration).
Viper 'UnmarshalKey' seems to have been designed to work on leaf
settings only. When being called on a group of settings, it would not
merge all sources (default, env vars, config, override, ...).

This means that using the `Set()` method could change the returned value
from 'UnmarshalKey' in unexpected ways (i.e. would no longer return the
value from the configuration).%

The second version of this fix correctly handles env vars override.
Copy link

👋 Thanks for contributing to Viper! You are awesome! 🎉

A maintainer will take a look at your pull request shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news,
either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

@dustmop dustmop closed this May 20, 2024
@CLAassistant
Copy link

CLAassistant commented May 20, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 15 committers have signed the CLA.

✅ albertvaka
✅ hush-hush
✅ gbbr
❌ olivielpeau
❌ xlucas
❌ Matthew Hardwick
❌ platinummonkey
❌ knusbaum
❌ mrhwick
❌ sgnn7
❌ mx-psi
❌ djmitche
❌ dustmop
❌ ganeshkumarsv
❌ pgimalac


Matthew Hardwick seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.