-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
[bug] merge_enabled=True
causes list values to merge
#999
Comments
merge_enabled
causes list values to mergemerge_enabled=True
causes list values to merge
This is actually not a bug, we can consider adding a new mode when implementing #299 Right now you need # -------------- config.toml --------------
[nested1.nested2]
value = [1, 2]
shouldnotchange = true
# -------------- new_config.toml --------------
[nested1.nested2]
value = [3]
dynaconf_merge = true |
I missed that on the Documentation. Thank you very much! Does it work even if i set only on the FIRST parsed configuration file? |
My problem is that the Any ideas? Otherwise i'll just try to avoid using lists and dictionaries in config files |
You can use https://www.dynaconf.com/configuration/?h=merge_enabled#merge_enabled |
Maybe I didn't get that right, but I'm already using |
Ohh sorry I overlooked your traceback, I answered based on the first error with So, right now using There is no way yet to set the merging strategy on a more granular way, this is being discussed on #299 and will probably be possible when we define an Schema on #683 So the current behavior is 8 or 80, or you need to specify the merge everywhere or it assumes merge everywhere. WorkaroundIn your case, I think the best option for now until we have custom merging strategies is to define a custom loader, |
Thank you! I'll try to give it a shot and if i manage to make something acceptable I'll post it here 😁 |
Thanks @rochacbruno, In the end I did it differently though, by exporting the current configuration as a dictionary and merging it to the new configuration (also exported as a dictionary to keep the key names equal) with a custom 💻 CodeThis is the code:
# -------------- mypackage/settings.py --------------
from mypackage.shared.merging import deepmerge
settings = Dynaconf(...)
def update_settings(filename: str | Path) -> None:
"""Update the settings using the given file.
Implements a custom merging strategy to merge the new
settings with the old ones (see [#999](https://github.com/dynaconf/dynaconf/issues/999)).
Args:
filename (str|Path): The path of the file to load.
"""python
new_settings = Dynaconf(settings_files=[filename]).to_dict()
old_settings = settings.to_dict()
new_settings_dict = deepmerge(old_settings, new_settings)
settings.update(new_settings_dict, merge=False, validate=True)
if __name__ == '__main__':
print(settings.nested1.nested2) # [1, 2]
update_settings("./new_config.toml")
print(settings.nested1.nested2) # [3] The custom merging function I used is the one from this SO answer:
# -------------- mypackage/shared/merging.py --------------
from typing import overload
from functools import reduce
from collections.abc import MutableMapping
@overload
def deepmerge(source: dict, destination: dict, /) -> dict:
"""Updates two dicts of dicts recursively.
If either mapping has leaves that are non-dicts, the
leaves of the second dictionary overwrite the ones
from the first.
Args:
source (dict): The source dictionary.
destination (dict): The destination dictionary.
"""
...
@overload
def deepmerge(*dicts) -> dict:
"""Updates multiple dicts of dicts recursively.
Args:
*dicts (dict): The dictionaries to merge.
"""
...
def deepmerge(*dicts) -> dict:
def _deepmerge(source: dict, destination: dict) -> dict:
"""Updates two dicts of dicts recursively (https://stackoverflow.com/a/24088493/8965861)."""
for k, v in source.items():
if k in destination:
# this next check is the only difference!
if all(isinstance(e, MutableMapping) for e in (v, destination[k])):
destination[k] = deepmerge(v, destination[k])
# we could further check types and merge as appropriate here.
d3 = source.copy()
d3.update(destination)
return d3
return reduce(_deepmerge, tuple(dicts)) 🧪 TestsFor the sake of completeness, here are the tests I used to make sure the import unittest
from veryeasyfatt.shared.merging import deepmerge
class MergerTestCase(unittest.TestCase):
# Do not use the docstring as the test name.
shortDescription = lambda self: None
def test_nochanges(self):
"""Test that the merger does not change the input in case the two dictionaries are equal."""
self.assertEqual(
deepmerge(
{
"a": 1,
"b": 2,
"c": {
"d": 3,
"e": 4,
"f": {
"g": 5,
"h": 6,
},
},
},
{
"a": 1,
"b": 2,
"c": {
"d": 3,
"e": 4,
"f": {
"g": 5,
"h": 6,
},
},
},
),
{
"a": 1,
"b": 2,
"c": {
"d": 3,
"e": 4,
"f": {
"g": 5,
"h": 6,
},
},
},
)
def test_replace(self):
"""The merger should replace the values in the first dictionary with the ones in the second."""
self.assertEqual(
deepmerge(
{
"a": 1,
"b": True,
},
{
"a": 2,
"b": False,
},
),
{
"a": 2,
"b": False,
},
)
def test_add(self):
"""The merger should add the values in the second dictionary to the first if they do not exist."""
self.assertEqual(
deepmerge(
{
"a": 1,
"b": 2,
},
{
"b": 2,
"c": 3,
"d": 4,
},
),
{
"a": 1,
"b": 2,
"c": 3,
"d": 4,
},
)
self.assertEqual(
deepmerge(
{
"a": 1,
"b": 2,
},
{
"c": 3,
"d": 4,
},
),
{
"a": 1,
"b": 2,
"c": 3,
"d": 4,
},
)
def test_lists(self):
"""The merger should merge lists correctly."""
self.assertEqual(
deepmerge(
{
"a": [1, 2, 3],
"b": 2,
},
{
"a": [1, 2, 3],
"b": 2,
},
),
{
"a": [1, 2, 3],
"b": 2,
},
)
self.assertEqual(
deepmerge(
{
"a": [1, 2, 3],
"b": 2,
},
{
"a": [1, 10, 3],
"b": 9,
},
),
{
"a": [1, 10, 3],
"b": 9,
},
)
def test_different_types(self):
"""The merger should merge different types without issues."""
self.assertEqual(
deepmerge(
{
"a": 1,
"b": 2,
},
{
"a": [1, 2, 3],
"b": True,
},
),
{
"a": [1, 2, 3],
"b": True,
},
)
self.assertEqual(
deepmerge(
{
"a": 1,
"b": {"c": 2},
},
{
"a": "1",
"b": True,
},
),
{
"a": "1",
"b": True,
},
)
if __name__ == "__main__":
unittest.main(verbosity=2) |
An even cleaner way to overcome the problem is to subclass the from dynaconf import Dynaconf as _Dynaconf, Validator
class Dynaconf(_Dynaconf):
def reload_settings(self, filename: str | Path) -> None:
"""Update the settings using the given file.
Implements a custom merging strategy to merge the new
settings with the old ones (see [#999](https://github.com/dynaconf/dynaconf/issues/999)).
Args:
filename (str|Path): The path of the file to load.
"""
new_settings = Dynaconf(settings_files=[filename]).to_dict()
old_settings = self.to_dict()
new_settings_dict = deepmerge(old_settings, new_settings)
self.update(new_settings_dict, merge=False, validate=True)
settings = Dynaconf(...) This way we can reload the configuration from anywhere with |
That is great @LukeSavefrogs lets see how we can add this kind of support when we implement #299 |
Problem
When
merge_enabled=True
on theDynaconf
object i would expect only the branches to be merged, not the leaves.Example
This code produces the following output:
merge_enabled=False
:merge_enabled=True
:merge_enabled=True
to do:Considerations
Essentially, I think that by default
merge_enabled=True
should only merge lists that do not contain basic types (str
,int
,float
,bool
, etc..).Doing this with dictionaries would be impossible without breaking backwards compatibility, but I think defining the config values through a Schema would help in solving this kind of problem since you could be able to define the merging strategy (
merge
orset
) via a property.Since the configuration files are also handled by users we should keep them as simple as possible, and using
@merge
and similar inside seems like it would confuse things to a lot of them.Similar issues
After reading through other issues I think this one is correlated to the following:
The text was updated successfully, but these errors were encountered: