Repository: IDSIA/sacred
Pull request #774 (2020-10-17 13:40:48)

Raise an exception if an unknown setting is accessed
This PR addresses #771 in that it freezes the settings structure and keys. It raises an exception if an unknown setting is accessed:

```python
from sacred import SETTINGS
SETTINGS.CONFIG.ASDF = 42
```
```
Traceback (most recent call last):
 ...
AttributeError: Unknown setting: ASDF. Possible keys are: dict_keys(['ENFORCE_KEYS_MONGO_COMPATIBLE', 'ENFORCE_KEYS_JSONPICKLE_COMPATIBLE', 'ENFORCE_VALID_PYTHON_IDENTIFIER_KEYS', 'ENFORCE_STRING_KEYS', 'ENFORCE_KEYS_NO_EQUALS', 'READ_ONLY_CONFIG', 'IGNORED_COMMENTS'])
```
Previously, it would just let the user set the invalid key.

stale[bot]: This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot]: This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


Patch:
diff --git a/dev-requirements.txt b/dev-requirements.txt
index 82ac3fa..791fc51 100644
--- a/dev-requirements.txt
+++ b/dev-requirements.txt
@@ -9,7 +9,7 @@ Mako
 MarkupSafe
 mock
 mongomock
-munch
+munch>=2.5
 packaging
 pandas
 pbr
diff --git a/requirements.txt b/requirements.txt
index bd53325..52a2f26 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,6 @@
 docopt>=0.3, <1.0
 jsonpickle>=1.2
-munch>=2.0.2, <3.0
+munch>=2.5, <3.0
 wrapt>=1.0, <2.0
 py-cpuinfo>=4.0
 colorama>=0.4
diff --git a/sacred/settings.py b/sacred/settings.py
index 426b575..7e146ad 100644
--- a/sacred/settings.py
+++ b/sacred/settings.py
@@ -2,14 +2,66 @@
 # coding=utf-8
 
 import platform
+from sacred.utils import SacredError
 import sacred.optional as opt
-from munch import munchify
+from munch import Munch
 from packaging import version
 
-__all__ = ("SETTINGS",)
+__all__ = ("SETTINGS", "SettingError")
 
 
-SETTINGS = munchify(
+class SettingError(SacredError):
+    """Error for invalid settings."""
+
+
+class FrozenKeyMunch(Munch):
+    __frozen_keys = False
+
+    def freeze_keys(self):
+        if self.__frozen_keys:
+            return
+        self.__frozen_keys = True
+        for v in self.values():
+            if isinstance(v, FrozenKeyMunch):
+                v.freeze_keys()
+
+    def _check_can_set(self, key, value):
+        if not self.__frozen_keys:
+            return
+
+        # Don't allow unknown keys
+        if key not in self:
+            raise SettingError(
+                f"Unknown setting: {key}. Possible keys are: " f"{list(self.keys())}"
+            )
+
+        # Don't allow setting keys that represent nested settings
+        if isinstance(self[key], Munch) and not isinstance(value, Munch):
+            # We don't want to overwrite a munch mapping. This is the easiest
+            # solution and closest to the original implementation where setting
+            # a setting with a dict would likely at some point cause an
+            # exception
+            raise SettingError(
+                f"Can't set this setting ({key}) to a non-munch value "
+                f"{value}, it is a nested setting!"
+            )
+
+    def __setitem__(self, key, value):
+        self._check_can_set(key, value)
+        super().__setitem__(key, value)
+
+    def __setattr__(self, key, value):
+        self._check_can_set(key, value)
+        super().__setattr__(key, value)
+
+    def __deepcopy__(self, memodict=None):
+        obj = self.__class__.fromDict(self.toDict())
+        if self.__frozen_keys:
+            obj.freeze_keys()
+        return obj
+
+
+SETTINGS = FrozenKeyMunch.fromDict(
     {
         "CONFIG": {
             # make sure all config keys are compatible with MongoDB
@@ -27,8 +79,8 @@ SETTINGS = munchify(
             # function are replaced with a read-only container that raises an
             # Exception if it is attempted to write to those containers
             "READ_ONLY_CONFIG": True,
-            # regex patterns to filter out certain IDE or linter directives from
-            # inline comments in the documentation
+            # regex patterns to filter out certain IDE or linter directives
+            # from inline comments in the documentation
             "IGNORED_COMMENTS": ["^pylint:", "^noinspection"],
             # if true uses the numpy legacy API, i.e. _rnd in captured functions is
             # a numpy.random.RandomState rather than numpy.random.Generator.
@@ -45,9 +97,11 @@ SETTINGS = munchify(
             "CAPTURED_ENV": [],
         },
         "COMMAND_LINE": {
-            # disallow string fallback, if parsing a value from command-line failed
+            # disallow string fallback, if parsing a value from command-line
+            # failed
             "STRICT_PARSING": False,
-            # show command line options that are disabled (e.g. unmet dependencies)
+            # show command line options that are disabled (e.g. unmet
+            # dependencies)
             "SHOW_DISABLED_OPTIONS": True,
         },
         # configure how stdout/stderr are captured. ['no', 'sys', 'fd']
@@ -56,5 +110,6 @@ SETTINGS = munchify(
         "DISCOVER_DEPENDENCIES": "imported",
         # configure how source-files are discovered. [none, imported, sys, dir]
         "DISCOVER_SOURCES": "imported",
-    }
+    },
 )
+SETTINGS.freeze_keys()


Test patch:
diff --git a/tests/test_settings.py b/tests/test_settings.py
new file mode 100644
index 0000000..5c5d464
--- /dev/null
+++ b/tests/test_settings.py
@@ -0,0 +1,28 @@
+import copy
+from sacred import SETTINGS as DEFAULT_SETTINGS
+
+import pytest
+
+from sacred.settings import SettingError
+
+
+def test_access_invalid_setting():
+    SETTINGS = copy.deepcopy(DEFAULT_SETTINGS)
+    with pytest.raises(SettingError):
+        SETTINGS.INVALID_SETTING = "invalid"
+    with pytest.raises(SettingError):
+        SETTINGS["INVALID_SETTING"] = "invalid"
+
+
+def test_overwrite_collection():
+    SETTINGS = copy.deepcopy(DEFAULT_SETTINGS)
+    with pytest.raises(SettingError):
+        SETTINGS.CONFIG = "invalid"
+    with pytest.raises(SettingError):
+        SETTINGS["CONFIG"] = "invalid"
+
+
+def test_access_valid_setting():
+    SETTINGS = copy.deepcopy(DEFAULT_SETTINGS)
+    SETTINGS.CONFIG.READ_ONLY_CONFIG = True
+    assert SETTINGS.CONFIG.READ_ONLY_CONFIG