From 46ec4a986f18447a170ba0401aea4f79f5b38aad Mon Sep 17 00:00:00 2001 From: Tim Savage Date: Wed, 20 Apr 2022 11:54:38 +1000 Subject: [PATCH 1/2] Improve reset settings to better restore and to properly re-initialise settings after reset --- src/pyapp/conf/__init__.py | 71 ++++++++++++++++++++++++++++---------- src/pyapp/conf/report.py | 5 ++- tests/conf/test_.py | 37 ++++++++++++++------ 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/src/pyapp/conf/__init__.py b/src/pyapp/conf/__init__.py index b17226de..85e6b6e2 100644 --- a/src/pyapp/conf/__init__.py +++ b/src/pyapp/conf/__init__.py @@ -93,14 +93,19 @@ import logging import os import warnings +from typing import Any +from typing import Iterable from typing import List from typing import Sequence +from typing import Tuple +from typing import Union from pyapp.conf import base_settings -from pyapp.conf.loaders import factory from pyapp.conf.loaders import Loader from pyapp.conf.loaders import ModuleLoader +from . import loaders + logger = logging.getLogger(__name__) DEFAULT_ENV_KEY = "PYAPP_SETTINGS" @@ -188,15 +193,25 @@ def reset_settings(self): This is useful for testing CLI entry points """ - setting_keys = [ - key for key in self._container.keys if key != "SETTINGS_SOURCES" - ] + container = self._container + + # Save and remove existing settings + saved_settings = [(key, container.__dict__.pop(key)) for key in container.keys] - for setting_key in setting_keys: - delattr(self, setting_key) + # Initialise base settings + container._populate_base_settings() # pylint: disable=protected-access - # Clear settings sources - setattr(self, "SETTINGS_SOURCES", []) + def restore_settings(): + # Remove new settings + for key in container.keys: + del container.__dict__[key] + + # Restore saved settings + container.__dict__.update(saved_settings) + + # Add restore action + action = restore_settings, () + self._roll_back.append(action) class Settings: @@ -205,14 +220,7 @@ class Settings: """ def __init__(self, base_settings_=None): - base_settings_ = base_settings_ or base_settings - - # Copy values from base settings file. - self.__dict__.update( - (k, getattr(base_settings_, k)) for k in dir(base_settings_) if k.upper() - ) - - self.__dict__["SETTINGS_SOURCES"] = [] # pylint: disable=invalid-name + self._populate_base_settings(base_settings_) def __getattr__(self, item): raise AttributeError("Setting not defined {!r}".format(item)) @@ -220,10 +228,22 @@ def __getattr__(self, item): def __setattr__(self, key, value): raise AttributeError("Readonly object") + def __getitem__(self, item): + return self.__dict__[item] + def __repr__(self) -> str: sources = self.SETTINGS_SOURCES or "UN-CONFIGURED" return f"{self.__class__.__name__}({sources})" + def _populate_base_settings(self, base_settings_=None): + base_settings_ = base_settings_ or base_settings + + # Copy values from base settings file. + self.__dict__.update( + (k, getattr(base_settings_, k)) for k in dir(base_settings_) if k.upper() + ) + self.__dict__["SETTINGS_SOURCES"] = [] # pylint: disable=invalid-name + @property def is_configured(self) -> bool: """ @@ -238,6 +258,14 @@ def keys(self) -> Sequence[str]: """ return [key for key in self.__dict__ if key.isupper()] + def items(self) -> Iterable[Tuple[str, Any]]: + """ + Return a sorted iterable of all key/value pairs of settings + """ + data = self.__dict__ + for key in sorted(self.keys): + yield key, data[key] + def load(self, loader: Loader, apply_method=None): """ Load settings from a loader instance. A loader is an iterator that yields key/value pairs. @@ -270,7 +298,7 @@ def load(self, loader: Loader, apply_method=None): include_settings = self.__dict__.pop("INCLUDE_SETTINGS", None) if include_settings: for source_url in include_settings: - self.load(factory(source_url), apply_method) + self.load(loaders.factory(source_url), apply_method) def load_from_loaders(self, loader_list: Sequence[Loader], override: bool = True): """ @@ -288,7 +316,7 @@ def load_from_loaders(self, loader_list: Sequence[Loader], override: bool = True def configure( self, - default_settings: Sequence[str], + default_settings: Union[str, Sequence[str]], runtime_settings: str = None, additional_loaders: Sequence[Loader] = None, env_settings_key: str = DEFAULT_ENV_KEY, @@ -304,13 +332,18 @@ def configure( """ logger.debug("Configuring settings...") + # Allow a simple string to be supplied + if isinstance(default_settings, str): + default_settings = [default_settings] + + # Build list of loaders loader_list: List[Loader] = [ModuleLoader(s) for s in default_settings] # Add run time settings (which can be overridden or specified by an # environment variable). runtime_settings = runtime_settings or os.environ.get(env_settings_key) if runtime_settings: - loader_list.append(ModuleLoader(runtime_settings)) + loader_list.append(loaders.factory(runtime_settings)) # Append the additional loaders if defined if additional_loaders: diff --git a/src/pyapp/conf/report.py b/src/pyapp/conf/report.py index e502e4a6..c8bee454 100644 --- a/src/pyapp/conf/report.py +++ b/src/pyapp/conf/report.py @@ -75,6 +75,5 @@ def run(self): """ Run the report """ - settings = self.settings - for key in settings.keys: - self.output_result(key, getattr(settings, key)) + for key, value in self.settings.items(): + self.output_result(key, value) diff --git a/tests/conf/test_.py b/tests/conf/test_.py index 305c9bc0..ebf0a546 100644 --- a/tests/conf/test_.py +++ b/tests/conf/test_.py @@ -6,7 +6,7 @@ class TestSettings: @pytest.fixture def target(self) -> pyapp.conf.Settings: target = pyapp.conf.Settings() - target.configure(["tests.settings"]) + target.configure("tests.settings") return target def test_ensure_readonly(self, target: pyapp.conf.Settings): @@ -20,6 +20,16 @@ def test_configure(self, target: pyapp.conf.Settings): assert not hasattr(target, "mixed_VALUE") def test_configure__from_runtime_parameter(self): + target = pyapp.conf.Settings() + target.configure("tests.settings", "tests.runtime_settings") + + assert "python:tests.runtime_settings" in target.SETTINGS_SOURCES + assert hasattr(target, "UPPER_VALUE") + assert hasattr(target, "RUNTIME_VALUE") + assert not hasattr(target, "lower_value") + assert not hasattr(target, "mixed_VALUE") + + def test_configure__with_a_list_of_settings(self): target = pyapp.conf.Settings() target.configure(["tests.settings"], "tests.runtime_settings") @@ -33,7 +43,7 @@ def test_configure__from_environment(self, monkeypatch): monkeypatch.setenv("PYAPP_SETTINGS", "tests.runtime_settings") target = pyapp.conf.Settings() - target.configure(["tests.settings"]) + target.configure("tests.settings") assert "python:tests.runtime_settings" in target.SETTINGS_SOURCES assert hasattr(target, "UPPER_VALUE") @@ -46,7 +56,7 @@ def test_configure__additional_loaders(self): with pytest.warns(ImportWarning): target.configure( - ["tests.settings"], + "tests.settings", "tests.runtime_settings", [pyapp.conf.ModuleLoader("tests.runtime_settings_with_imports")], ) @@ -56,14 +66,14 @@ def test_configure__additional_loaders(self): def test_load__duplicate_settings_file(self): target = pyapp.conf.Settings() - target.configure(["tests.settings"], "tests.runtime_settings") + target.configure("tests.settings", "tests.runtime_settings") with pytest.warns(ImportWarning): target.load(pyapp.conf.ModuleLoader("tests.runtime_settings")) def test_load__specify_include_settings(self): target = pyapp.conf.Settings() - target.configure(["tests.settings"], "tests.runtime_settings_with_imports") + target.configure("tests.settings", "tests.runtime_settings_with_imports") assert "python:tests.runtime_settings_with_imports" in target.SETTINGS_SOURCES assert "python:tests.runtime_settings" in target.SETTINGS_SOURCES @@ -142,13 +152,20 @@ def test_modify__reset_settings(self, target: pyapp.conf.Settings): "TEST_PROVIDERS", } + initial_keys = target.keys + with target.modify() as patch: patch.reset_settings() - assert all(not hasattr(target, key) for key in known_keys) - assert target.SETTINGS_SOURCES == [] - assert not target.is_configured + assert all( + not hasattr(target, key) for key in known_keys + ), "Custom keys still exist" + assert target.SETTINGS_SOURCES == [], "Sources have not been cleared" + assert not target.is_configured, "Is still listed as configured" + assert isinstance(target.LOGGING, dict), "Base settings missing" # Check items have been restored - assert all(hasattr(target, key) for key in known_keys) - assert target.SETTINGS_SOURCES == ["python:tests.settings"] + assert initial_keys == target.keys, "All settings not restored" + assert target.SETTINGS_SOURCES == [ + "python:tests.settings" + ], "Sources not restored" From 6ec3fd40c27c889cebbd591238d5ebe05a6ec306 Mon Sep 17 00:00:00 2001 From: Tim Savage Date: Wed, 20 Apr 2022 11:56:03 +1000 Subject: [PATCH 2/2] Update version and history --- HISTORY | 11 +++++++++++ pyproject.toml | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/HISTORY b/HISTORY index a5117389..649161a8 100644 --- a/HISTORY +++ b/HISTORY @@ -1,3 +1,14 @@ +4.7.1 +===== + +Changes +-------- + +- Improve the settings reset to properly apply the base-settings after reset. + These are the basic settings required for the application to operate + correctly. + + 4.7.0 ===== diff --git a/pyproject.toml b/pyproject.toml index e8abce0f..0e39621a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "rtd_poetry" [tool.poetry] name = "pyapp" -version = "4.7.0" +version = "4.7.1" description = "A Python application framework - Let us handle the boring stuff!" authors = ["Tim Savage "] license = "BSD-3-Clause"