From da05a93e03d983cc9807c0030031e655c4b892fa Mon Sep 17 00:00:00 2001 From: Cedric Buissart Date: Fri, 20 Oct 2023 02:39:24 +0200 Subject: [PATCH] [configModel] Create path when it is partial (#150) This kind of YAML was causing trouble: ```yaml a: ``` When reading this, it is converted to `{"a": None}`, but configModel would refuse to go down that path to set a value (unless we overwrite). This change fixes that: in this condition, config.set() no longer consider setting `a.b` as overwriting `None` to a dictionary This fixes the ZAP template configuration, where the following happened: ```yaml general: container: ``` the code would refuse to set `container.parameters.executable` because container existed but points to `None` This resulted in `zap.sh` not being set --- configmodel/__init__.py | 6 +++++- tests/configmodel/test_configmodel.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/configmodel/__init__.py b/configmodel/__init__.py index 96355c28..d4859d30 100644 --- a/configmodel/__init__.py +++ b/configmodel/__init__.py @@ -116,7 +116,11 @@ def set(self, path, value, overwrite=True): walk = walk[key] continue tmp = walk[key] - # case 2: not a "dictionary" type: warn and overwrite (if True) + # case 2: the value is None (path partially exists): initialize a dictionary + if tmp is None: + walk[key] = {} + tmp = walk[key] + # case 3: not a "dictionary" type: warn and overwrite (if True) if not isinstance(tmp, dict): logging.warning( f"RapidastConfigModel.set: Incompatible {path} at {tmp}" diff --git a/tests/configmodel/test_configmodel.py b/tests/configmodel/test_configmodel.py index 79f16710..06f95f8a 100644 --- a/tests/configmodel/test_configmodel.py +++ b/tests/configmodel/test_configmodel.py @@ -27,6 +27,7 @@ def generate_some_nested_config(): "key4": "value4", "nested": {"morenested": {"key3": "nestedvalue"}}, "nothing": None, + "falsekey": False, } @@ -87,6 +88,18 @@ def test_configmodel_set(some_nested_config): myconf.set("nested.morenested", "mynewval", overwrite=True) assert myconf.get("nested.morenested") == "mynewval" + # deep sets + # `set` should rewrite a `None` value even on `overwrite=False` + myconf.set("nothing.some.more.nested", "newval", overwrite=False) + assert myconf.get("nothing.some.more.nested") == "newval" + # but should not rewrite a value set to False value + myconf.set("falsekey.some.more.nested", "newval", overwrite=False) + assert not myconf.exists("falsekey.some.more.nested") + # unless we overwrite + myconf.set("falsekey.some.more.nested", "newval", overwrite=True) + assert myconf.exists("falsekey.some.more.nested") + assert myconf.get("falsekey.some.more.nested") == "newval" + def test_configmodel_move(some_nested_config): myconf = RapidastConfigModel(some_nested_config)