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

feature request: control merging behavior #62

Closed
grount opened this issue Feb 18, 2021 · 6 comments
Closed

feature request: control merging behavior #62

grount opened this issue Feb 18, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@grount
Copy link
Contributor

grount commented Feb 18, 2021

When a user loads multiple YAML files that have the same key with different types the last type loaded is deciding the loaded type.
For example:
first.yml

key: value

second.yml

key: [1,2,3]

The loaded value into the key field will be [1,2,3] if second.yml will be loaded last.
I would like to control the behavior that it could be "loss" with the current behavior and "strict" with type enforcing in the merge.

If it makes sense, I could try to work on it with some guidance 👍
Or if it already exists, I'd appreciate the 'how-to'.

@knadh
Copy link
Owner

knadh commented Feb 18, 2021

Yep, the current behaviour is to overwrite the previous values in subsequent merges. To add type checks and other similar features, roughly:

  • Add a NewWithConf(delim, koanf.Conf{}) that takes a new koanf.Conf{} that can contain arbitrary feature flags.
  • koanf.Config { bool StrictMerge }
  • Make merge() behave differently based on this config value.
  • Will probably need a MergeStrict() in the maps package as well.

@knadh knadh added the enhancement New feature or request label Feb 18, 2021
@grount
Copy link
Contributor Author

grount commented Feb 18, 2021

Thank you for the quick response!
Should I merge the delim field in the new koanf.Config?

@knadh
Copy link
Owner

knadh commented Feb 18, 2021

Ah yes, makes sense.

@grount
Copy link
Contributor Author

grount commented Feb 21, 2021

maps.MergeStrict has to return an error since the merging process can fail.
In koanf, merge is used in 4 places:

  1. Load - can return an error.
  2. Cut - cannot return an error - but okay since it's a subset.
  3. Merge - cannot return an error - possible mergeStrict errors.
  4. MergeAt - cannot return an error - possible mergeStrict errors.

As I see it since the koanf.Conf has MergeStrict: true it should affect all the merging behavior across koanf.

Possible solutions:

  1. We can either exclude koanf.Merge and koanf.MergeAt by adding koanf.MergeStrict and koanf.MergeStrictAt and ignore the koanf.Conf
  2. We can ignore MergeStrict flag in koanf.Merge and koanf.MergeAt functions
  3. Change koanf.Merge and koanf.MergeAt signatures so it will return an error. I think it will not break backward compatibility.

I'm up for number 3, WDYT?

@knadh
Copy link
Owner

knadh commented Feb 21, 2021

3 is the cleanest approach. We can bump the version to 1.0.0 after this. Long overdue.

@grount
Copy link
Contributor Author

grount commented Feb 22, 2021

Merging between JSON and YAML will fail because of the float64/int issue.
We can either say that you cannot MergeStrict between YAML and JSON and the recommended way is to use mergeStrict between the same types, or we can check if float64 is a whole number and convert to int but it will not cover cases where user-defined a whole float number: 3.0 but we can only patch one case where in the future there could be more of them.

grount added a commit to grount/koanf that referenced this issue Feb 23, 2021
grount added a commit to grount/koanf that referenced this issue Feb 23, 2021
grount added a commit to grount/koanf that referenced this issue Feb 23, 2021
grount added a commit to grount/koanf that referenced this issue Feb 23, 2021
@knadh knadh closed this as completed Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants