From dd534d90954ff0c2665075dc46e9d3424b917e0f Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Fri, 24 Feb 2023 11:33:25 +0100 Subject: [PATCH] Feature: support for env vars reading in config --- notebooks/Quick Intro.ipynb | 25 ++++++++++++++++++ src/accsr/config.py | 15 ++++++++--- tests/accsr/conftest.py | 6 +---- tests/accsr/resources/config.json | 1 + tests/accsr/test_config.py | 42 ++++++++++++++++++++++++++++++- 5 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 tests/accsr/resources/config.json diff --git a/notebooks/Quick Intro.ipynb b/notebooks/Quick Intro.ipynb index 7f9d246..8c2c20a 100644 --- a/notebooks/Quick Intro.ipynb +++ b/notebooks/Quick Intro.ipynb @@ -136,6 +136,31 @@ "the developers who need to update data." ] }, + { + "cell_type": "markdown", + "source": [ + "### Including environment variables\n", + "\n", + "One can tell the configuration to read the value off an environment variable instead of writing\n", + "the value directly to the file. This is useful for example for running code in CI, where\n", + "it might be easier to adjust environment variables instead of files (for example, while\n", + "Gitlab CI offers file-type secrets, there is no such feature in GitHub actions at the time of writing).\n", + "\n", + "For instructing to read off the value from the env, simply prepend \"env:\" to the configured value,\n", + "e.g. like this in your `config.json`:\n", + "\n", + "\n", + "```json\n", + "{\n", + " ...\n", + " \"my_env_var\": \"env:MY_ENV_VAR\"\n", + "}\n", + "```" + ], + "metadata": { + "collapsed": false + } + }, { "cell_type": "markdown", "metadata": {}, diff --git a/src/accsr/config.py b/src/accsr/config.py index 412f967..6cf34d0 100644 --- a/src/accsr/config.py +++ b/src/accsr/config.py @@ -51,13 +51,15 @@ class ConfigurationBase(ABC): instead inherit from it. """ + ENV_VAR_MARKER = "env:" + def __init__( self, config_directory: str = None, config_files=("config.json", "config_local.json"), ): """ - :param config_directory: directory where to look for the config files. Typically this will be a project's + :param config_directory: directory where to look for the config files. Typically, this will be a project's root directory. If None, the directory with the module containing the configuration class definition (inherited from ConfigurationBase) will be used. :param config_files: list of JSON configuration files (relative to config_directory) from which to read. @@ -101,7 +103,14 @@ def _get_non_empty_entry( for k in key: value = value.get(k) if value is None: - raise Exception(f"Value for key '{key}' not set in configuration") + raise KeyError(f"Value for key '{key}' not set in configuration") + + # Special case allowing to extract values from env vars + if isinstance(value, str) and value.startswith(self.ENV_VAR_MARKER): + env_var_name = value.lstrip(self.ENV_VAR_MARKER) + value = os.getenv(env_var_name) + if value is None: + raise KeyError(f"Expected non-empty env var: {env_var_name}.") return value def _get_existing_path(self, key: Union[str, List[str]], create=True) -> str: @@ -286,7 +295,7 @@ def get_config(self, reload=False, *args, **kwargs) -> ConfigurationClass: """ Retrieves the configuration object (as singleton). - :param reload: if True, the config will be reloaded from disk even if it a configuration object already exists. + :param reload: if True, the config will be reloaded from disk even if a configuration object already exists. This is mainly useful in interactive environments like notebooks :param args: passed to init of the configuration class :param kwargs: passed to init of the configuration class constructor diff --git a/tests/accsr/conftest.py b/tests/accsr/conftest.py index 844af15..f1d5766 100644 --- a/tests/accsr/conftest.py +++ b/tests/accsr/conftest.py @@ -10,14 +10,10 @@ from accsr.remote_storage import RemoteStorage, RemoteStorageConfig -top_level_directory = os.path.dirname(__file__) - -TEST_RESOURCES = os.path.join(top_level_directory, "resources") - @pytest.fixture(scope="session") def test_resources(): - return TEST_RESOURCES + return os.path.join(os.path.dirname(__file__), "resources") @pytest.fixture(scope="session") diff --git a/tests/accsr/resources/config.json b/tests/accsr/resources/config.json new file mode 100644 index 0000000..2b9572c --- /dev/null +++ b/tests/accsr/resources/config.json @@ -0,0 +1 @@ +{"env_var_entry": "env:THIS_EXISTS", "empty_env_var_entry": "env:THIS_EXISTS_NOT"} diff --git a/tests/accsr/test_config.py b/tests/accsr/test_config.py index 65e8972..73be163 100644 --- a/tests/accsr/test_config.py +++ b/tests/accsr/test_config.py @@ -1,6 +1,34 @@ +import os + +import pytest + +from accsr.config import ConfigProviderBase, DefaultDataConfiguration from accsr.remote_storage import RemoteStorageConfig +class __Configuration(DefaultDataConfiguration): + # has to be kept in sync with resources/config.json + @property + def env_var_entry(self): + return self._get_non_empty_entry("env_var_entry") + + @property + def empty_env_var_entry(self): + return self._get_non_empty_entry("empty_env_var_entry") + + +class ConfigProvider(ConfigProviderBase[__Configuration]): + pass + + +_config_provider = ConfigProvider() + + +@pytest.fixture() +def test_config(test_resources, reload=False): + return _config_provider.get_config(reload=reload, config_directory=test_resources) + + def test_storage_config_repr_does_not_include_secret(): """ Ensure that str representation of storage config does not leak secret. @@ -8,8 +36,20 @@ def test_storage_config_repr_does_not_include_secret(): Regression test for issue #6. """ cfg = RemoteStorageConfig( - "provider", "key", "bucket", "secretkey", "region", "host", "1234", "base_path" + "provider", "key", "bucket", "secretkey", "region", "host", 1234, "base_path" ) assert cfg.secret not in repr(cfg) assert cfg.secret not in str(cfg) + + +class TestConfig: + def test_env_var_retrieval(self, test_config): + os.environ["THIS_EXISTS"] = "env_entry" + assert test_config.env_var_entry == "env_entry" + + def test_empty_env_var_raises(self, test_config): + os.environ.pop("THIS_EXISTS_NOT", None) + with pytest.raises(KeyError) as e: + test_config.empty_env_var_entry + assert "THIS_EXISTS_NOT" in str(e)