From 78beaf8fb23c29c95c9215181585ed43b8772e69 Mon Sep 17 00:00:00 2001 From: Ricardo Garcia Silva Date: Wed, 20 Mar 2024 08:33:33 +0000 Subject: [PATCH 1/5] Allow reading default value for variable in config --- pygeoapi/util.py | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/pygeoapi/util.py b/pygeoapi/util.py index 280362d1a..4aed107f4 100644 --- a/pygeoapi/util.py +++ b/pygeoapi/util.py @@ -163,16 +163,33 @@ def yaml_load(fh: IO) -> dict: :returns: `dict` representation of YAML """ - # support environment variables in config - # https://stackoverflow.com/a/55301129 - path_matcher = re.compile(r'.*\$\{([^}^{]+)\}.*') + # # support environment variables in config + # # https://stackoverflow.com/a/55301129 + + path_matcher = re.compile( + r'\$\{?(?P\w+)(:-(?P[^}]+))?\}?') def path_constructor(loader, node): - env_var = path_matcher.match(node.value).group(1) - if env_var not in os.environ: - msg = f'Undefined environment variable {env_var} in config' - raise EnvironmentError(msg) - return get_typed_value(os.path.expandvars(node.value)) + result = "" + current_index = 0 + raw_value = node.value + print(f"{raw_value=}") + for match_obj in path_matcher.finditer(raw_value): + groups = match_obj.groupdict() + result += raw_value[current_index:match_obj.start()] + if (var_value := os.getenv(groups['varname'])) is not None: + result += var_value + elif (default_value := groups.get('default')) is not None: + result += default_value + else: + raise RuntimeError( + f'Could not find the {groups["varname"]!r} environment ' + f'variable' + ) + current_index = match_obj.end() + else: + result += raw_value[current_index:] + return get_typed_value(result) class EnvVarLoader(yaml.SafeLoader): pass From 3b25a1f39639e0946c84d6815f8e8fc2fb49e421 Mon Sep 17 00:00:00 2001 From: Ricardo Garcia Silva Date: Wed, 20 Mar 2024 16:33:39 +0000 Subject: [PATCH 2/5] Added tests and documentation regarding default env variables in the config file --- docs/source/configuration.rst | 22 +++++++++++++++++++++ pygeoapi/util.py | 8 +++----- tests/test_util.py | 36 +++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index b9422d828..401830030 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -413,6 +413,28 @@ Below is an example of how to integrate system environment variables in pygeoapi port: ${MY_PORT} +It is also possible to define a default value for a variable in case it does not exist in +the environment using a syntax like: ``value: ${ENV_VAR:-the default}`` + +.. code-block:: yaml + + server: + bind: + host: ${MY_HOST:-localhost} + port: ${MY_PORT:-5000} + + +If you need to refer to an environment variable in middle of the variable you must use the ``!env`` YAML tag, +as shown in the example below: + +.. code-block:: yaml + + metadata: + identification: + title: + en: !env This is pygeoapi host ${MY_HOST} and port ${MY_PORT:-5000}, nice to meet you! + + Hierarchical collections ------------------------ diff --git a/pygeoapi/util.py b/pygeoapi/util.py index 4aed107f4..33d3c0fb9 100644 --- a/pygeoapi/util.py +++ b/pygeoapi/util.py @@ -167,13 +167,12 @@ def yaml_load(fh: IO) -> dict: # # https://stackoverflow.com/a/55301129 path_matcher = re.compile( - r'\$\{?(?P\w+)(:-(?P[^}]+))?\}?') + r'\$\{(?P\w+)(:-(?P[^}]+))?\}') def path_constructor(loader, node): result = "" current_index = 0 raw_value = node.value - print(f"{raw_value=}") for match_obj in path_matcher.finditer(raw_value): groups = match_obj.groupdict() result += raw_value[current_index:match_obj.start()] @@ -194,9 +193,8 @@ def path_constructor(loader, node): class EnvVarLoader(yaml.SafeLoader): pass - EnvVarLoader.add_implicit_resolver('!path', path_matcher, None) - EnvVarLoader.add_constructor('!path', path_constructor) - + EnvVarLoader.add_implicit_resolver('!env', path_matcher, None) + EnvVarLoader.add_constructor('!env', path_constructor) return yaml.load(fh, Loader=EnvVarLoader) diff --git a/tests/test_util.py b/tests/test_util.py index 98097f5ab..e3b7cdf19 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -31,6 +31,8 @@ from decimal import Decimal from contextlib import nullcontext as does_not_raise from copy import deepcopy +from io import StringIO +from unittest import mock import pytest from pyproj.exceptions import CRSError @@ -77,6 +79,40 @@ def test_yaml_load(config): util.yaml_load(fh) +@pytest.mark.parametrize('env,input_config,expected', [ + pytest.param({}, 'foo: something', {'foo': 'something'}, id='no-env-expansion'), # noqa + pytest.param({'FOO': 'this'}, 'foo: ${FOO}', {'foo': 'this'}), # noqa + pytest.param({'FOO': 'this'}, 'foo: !env the value is ${FOO}', {'foo': 'the value is this'}, id='explicit-yaml-tag'), # noqa + pytest.param({}, 'foo: ${FOO:-some default}', {'foo': 'some default'}), # noqa + pytest.param({'FOO': 'this', 'BAR': 'that'}, 'composite: ${FOO}:${BAR}', {'composite': 'this:that'}), # noqa + pytest.param({}, 'composite: ${FOO:-default-foo}:${BAR:-default-bar}', {'composite': 'default-foo:default-bar'}), # noqa + pytest.param( + { + 'HOST': 'fake-host', + 'USER': 'fake', + 'PASSWORD': 'fake-pass', + 'DB': 'fake-db' + }, + 'connection: !env "postgres://${USER}:${PASSWORD}@${HOST}:${PORT:-5432}/${DB}"', + { + 'connection': 'postgres://fake:fake-pass@fake-host:5432/fake-db' + }, + id='multiple-yaml-tag' + ), +]) +def test_yaml_load_with_env_variables( + env: dict[str, str], input_config: str, expected): + + def mock_get_env(env_var_name): + result = env.get(env_var_name) + return result + + with mock.patch('pygeoapi.util.os') as mock_os: + mock_os.getenv.side_effect = mock_get_env + loaded_config = util.yaml_load(StringIO(input_config)) + assert loaded_config == expected + + def test_str2bool(): assert not util.str2bool(False) assert not util.str2bool('0') From 3247b4c0fa655a41bae2c61e5613feead4950a26 Mon Sep 17 00:00:00 2001 From: Ricardo Garcia Silva Date: Wed, 20 Mar 2024 16:35:24 +0000 Subject: [PATCH 3/5] flake8 fixes --- tests/test_util.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_util.py b/tests/test_util.py index e3b7cdf19..762d57297 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -80,12 +80,12 @@ def test_yaml_load(config): @pytest.mark.parametrize('env,input_config,expected', [ - pytest.param({}, 'foo: something', {'foo': 'something'}, id='no-env-expansion'), # noqa - pytest.param({'FOO': 'this'}, 'foo: ${FOO}', {'foo': 'this'}), # noqa - pytest.param({'FOO': 'this'}, 'foo: !env the value is ${FOO}', {'foo': 'the value is this'}, id='explicit-yaml-tag'), # noqa - pytest.param({}, 'foo: ${FOO:-some default}', {'foo': 'some default'}), # noqa - pytest.param({'FOO': 'this', 'BAR': 'that'}, 'composite: ${FOO}:${BAR}', {'composite': 'this:that'}), # noqa - pytest.param({}, 'composite: ${FOO:-default-foo}:${BAR:-default-bar}', {'composite': 'default-foo:default-bar'}), # noqa + pytest.param({}, 'foo: something', {'foo': 'something'}, id='no-env-expansion'), # noqa E501 + pytest.param({'FOO': 'this'}, 'foo: ${FOO}', {'foo': 'this'}), # noqa E501 + pytest.param({'FOO': 'this'}, 'foo: !env the value is ${FOO}', {'foo': 'the value is this'}, id='explicit-yaml-tag'), # noqa E501 + pytest.param({}, 'foo: ${FOO:-some default}', {'foo': 'some default'}), # noqa E501 + pytest.param({'FOO': 'this', 'BAR': 'that'}, 'composite: ${FOO}:${BAR}', {'composite': 'this:that'}), # noqa E501 + pytest.param({}, 'composite: ${FOO:-default-foo}:${BAR:-default-bar}', {'composite': 'default-foo:default-bar'}), # noqa E501 pytest.param( { 'HOST': 'fake-host', @@ -93,7 +93,7 @@ def test_yaml_load(config): 'PASSWORD': 'fake-pass', 'DB': 'fake-db' }, - 'connection: !env "postgres://${USER}:${PASSWORD}@${HOST}:${PORT:-5432}/${DB}"', + 'connection: !env "postgres://${USER}:${PASSWORD}@${HOST}:${PORT:-5432}/${DB}"', # noqa E501 { 'connection': 'postgres://fake:fake-pass@fake-host:5432/fake-db' }, From eea9db8b8af7a9932322d19dd9933942c6a60bbb Mon Sep 17 00:00:00 2001 From: Ricardo Garcia Silva Date: Thu, 21 Mar 2024 11:06:00 +0000 Subject: [PATCH 4/5] Improve regular expression and fix failing tests --- docs/source/configuration.rst | 9 +-------- pygeoapi/util.py | 17 +++++++++-------- tests/test_util.py | 6 +++--- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index 401830030..a0c05a7d0 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -422,17 +422,10 @@ the environment using a syntax like: ``value: ${ENV_VAR:-the default}`` bind: host: ${MY_HOST:-localhost} port: ${MY_PORT:-5000} - - -If you need to refer to an environment variable in middle of the variable you must use the ``!env`` YAML tag, -as shown in the example below: - -.. code-block:: yaml - metadata: identification: title: - en: !env This is pygeoapi host ${MY_HOST} and port ${MY_PORT:-5000}, nice to meet you! + en: This is pygeoapi host ${MY_HOST} and port ${MY_PORT:-5000}, nice to meet you! Hierarchical collections diff --git a/pygeoapi/util.py b/pygeoapi/util.py index 33d3c0fb9..2ccbeb25f 100644 --- a/pygeoapi/util.py +++ b/pygeoapi/util.py @@ -166,22 +166,23 @@ def yaml_load(fh: IO) -> dict: # # support environment variables in config # # https://stackoverflow.com/a/55301129 - path_matcher = re.compile( - r'\$\{(?P\w+)(:-(?P[^}]+))?\}') + env_matcher = re.compile( + r'.*?\$\{(?P\w+)(:-(?P[^}]+))?\}') - def path_constructor(loader, node): + def env_constructor(loader, node): result = "" current_index = 0 raw_value = node.value - for match_obj in path_matcher.finditer(raw_value): + for match_obj in env_matcher.finditer(raw_value): groups = match_obj.groupdict() - result += raw_value[current_index:match_obj.start()] + varname_start = match_obj.span('varname')[0] + result += raw_value[current_index:(varname_start-2)] if (var_value := os.getenv(groups['varname'])) is not None: result += var_value elif (default_value := groups.get('default')) is not None: result += default_value else: - raise RuntimeError( + raise EnvironmentError( f'Could not find the {groups["varname"]!r} environment ' f'variable' ) @@ -193,8 +194,8 @@ def path_constructor(loader, node): class EnvVarLoader(yaml.SafeLoader): pass - EnvVarLoader.add_implicit_resolver('!env', path_matcher, None) - EnvVarLoader.add_constructor('!env', path_constructor) + EnvVarLoader.add_implicit_resolver('!env', env_matcher, None) + EnvVarLoader.add_constructor('!env', env_constructor) return yaml.load(fh, Loader=EnvVarLoader) diff --git a/tests/test_util.py b/tests/test_util.py index 762d57297..262cfcd8a 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -82,7 +82,7 @@ def test_yaml_load(config): @pytest.mark.parametrize('env,input_config,expected', [ pytest.param({}, 'foo: something', {'foo': 'something'}, id='no-env-expansion'), # noqa E501 pytest.param({'FOO': 'this'}, 'foo: ${FOO}', {'foo': 'this'}), # noqa E501 - pytest.param({'FOO': 'this'}, 'foo: !env the value is ${FOO}', {'foo': 'the value is this'}, id='explicit-yaml-tag'), # noqa E501 + pytest.param({'FOO': 'this'}, 'foo: the value is ${FOO}', {'foo': 'the value is this'}, id='no-need-for-yaml-tag'), # noqa E501 pytest.param({}, 'foo: ${FOO:-some default}', {'foo': 'some default'}), # noqa E501 pytest.param({'FOO': 'this', 'BAR': 'that'}, 'composite: ${FOO}:${BAR}', {'composite': 'this:that'}), # noqa E501 pytest.param({}, 'composite: ${FOO:-default-foo}:${BAR:-default-bar}', {'composite': 'default-foo:default-bar'}), # noqa E501 @@ -93,11 +93,11 @@ def test_yaml_load(config): 'PASSWORD': 'fake-pass', 'DB': 'fake-db' }, - 'connection: !env "postgres://${USER}:${PASSWORD}@${HOST}:${PORT:-5432}/${DB}"', # noqa E501 + 'connection: postgres://${USER}:${PASSWORD}@${HOST}:${PORT:-5432}/${DB}', # noqa E501 { 'connection': 'postgres://fake:fake-pass@fake-host:5432/fake-db' }, - id='multiple-yaml-tag' + id='multiple-no-need-yaml-tag' ), ]) def test_yaml_load_with_env_variables( From c54fab26a5d60a9506b0a4f3f2330dc85387a079 Mon Sep 17 00:00:00 2001 From: Ricardo Garcia Silva Date: Fri, 22 Mar 2024 20:49:57 +0000 Subject: [PATCH 5/5] Added additional example to docs --- docs/source/configuration.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index a0c05a7d0..b11cfdd5f 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -412,6 +412,11 @@ Below is an example of how to integrate system environment variables in pygeoapi host: ${MY_HOST} port: ${MY_PORT} +Multiple environment variables are supported as follows: + +.. code-block:: yaml + + data: ${MY_HOST}:${MY_PORT} It is also possible to define a default value for a variable in case it does not exist in the environment using a syntax like: ``value: ${ENV_VAR:-the default}``