From 07381e147979e9a0ab38cfe174e3a05f73154516 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Fri, 1 Mar 2024 21:17:13 +0100 Subject: [PATCH] Issue #112 eliminate AggregatorConfig loading related logic --- .../background/prime_caches.py | 4 +- src/openeo_aggregator/config.py | 61 ------------------- src/openeo_aggregator/metadata/validator.py | 7 +-- tests/test_config.py | 53 +--------------- 4 files changed, 5 insertions(+), 120 deletions(-) diff --git a/src/openeo_aggregator/background/prime_caches.py b/src/openeo_aggregator/background/prime_caches.py index d203783..04dd2e0 100644 --- a/src/openeo_aggregator/background/prime_caches.py +++ b/src/openeo_aggregator/background/prime_caches.py @@ -20,7 +20,7 @@ from openeo_aggregator.about import log_version_info from openeo_aggregator.app import get_aggregator_logging_config from openeo_aggregator.backend import AggregatorBackendImplementation -from openeo_aggregator.config import OPENEO_AGGREGATOR_CONFIG, get_backend_config +from openeo_aggregator.config import get_backend_config from openeo_aggregator.connection import MultiBackendConnection _log = logging.getLogger(__name__) @@ -37,7 +37,7 @@ def main(args: Optional[List[str]] = None): # TODO #112 remove this unused CLI option "--config", default=None, - help=f"Optional: aggregator config to load (instead of env var {OPENEO_AGGREGATOR_CONFIG} based resolution).", + help=f"Optional: aggregator config to load (instead of env var based resolution).", ) cli.add_argument( "--require-zookeeper-writes", diff --git a/src/openeo_aggregator/config.py b/src/openeo_aggregator/config.py index 4dc6602..80de62b 100644 --- a/src/openeo_aggregator/config.py +++ b/src/openeo_aggregator/config.py @@ -15,7 +15,6 @@ _log = logging.getLogger(__name__) -OPENEO_AGGREGATOR_CONFIG = "OPENEO_AGGREGATOR_CONFIG" CACHE_TTL_DEFAULT = 6 * 60 * 60 @@ -45,66 +44,6 @@ class AggregatorConfig(dict): # Just a config field for test purposes (while were stripping down this config class) test_dummy = dict_item(default="alice") - @staticmethod - def from_py_file(path: Union[str, Path]) -> 'AggregatorConfig': - """Load config from Python file.""" - path = Path(path) - _log.debug(f"Loading config from Python file {path}") - # Based on flask's Config.from_pyfile - with path.open(mode="rb") as f: - code = compile(f.read(), path, "exec") - globals = {"__file__": str(path)} - exec(code, globals) - for var_name in ["aggregator_config", "config"]: - if var_name in globals: - config = globals[var_name] - _log.info(f"Loaded config from {path=} {var_name=}") - break - else: - raise ConfigException(f"No 'config' variable defined in config file {path}") - if not isinstance(config, AggregatorConfig): - raise ConfigException(f"Variable 'config' from {path} is not AggregatorConfig but {type(config)}") - return config - - def copy(self) -> 'AggregatorConfig': - return AggregatorConfig(self) - - -def get_config_dir() -> Path: - # TODO: make this robust against packaging operations (e.g. no guaranteed real __file__ path) - # TODO #117: eliminate the need for this hardcoded, package based config dir - for root in [Path.cwd(), Path(__file__).parent.parent.parent]: - config_dir = root / "conf" - if config_dir.is_dir(): - return config_dir - raise RuntimeError("No config dir found") - - -def get_config(x: Union[str, Path, AggregatorConfig, None] = None) -> AggregatorConfig: - """ - Get aggregator config from given object: - - if None: check env variable "OPENEO_AGGREGATOR_CONFIG" or return default config - - if it is already an `AggregatorConfig` object: return as is - - if it is a string and looks like a path of a Python file: load config from that - """ - - if x is None: - if OPENEO_AGGREGATOR_CONFIG in os.environ: - x = os.environ[OPENEO_AGGREGATOR_CONFIG] - _log.info(f"Config from env var {OPENEO_AGGREGATOR_CONFIG}: {x!r}") - else: - x = get_config_dir() / "aggregator.dummy.py" - _log.info(f"Config from fallback {x!r}") - - if isinstance(x, str): - x = Path(x) - - if isinstance(x, AggregatorConfig): - return x - elif isinstance(x, Path) and x.suffix.lower() == ".py": - return AggregatorConfig.from_py_file(x) - - raise ValueError(repr(x)) @attrs.frozen(kw_only=True) diff --git a/src/openeo_aggregator/metadata/validator.py b/src/openeo_aggregator/metadata/validator.py index 18653db..e8dc04d 100644 --- a/src/openeo_aggregator/metadata/validator.py +++ b/src/openeo_aggregator/metadata/validator.py @@ -1,10 +1,10 @@ import argparse import logging -from typing import Dict, List +from typing import Dict import requests -from openeo_aggregator.config import AggregatorConfig, get_backend_config, get_config +from openeo_aggregator.config import get_backend_config from openeo_aggregator.metadata.merging import ( ProcessMetadataMerger, merge_collection_metadata, @@ -45,9 +45,6 @@ def main(): logging.basicConfig(level=logging.DEBUG if args.verbose else logging.WARNING) _log.info(f"{args=}") - # Load config - # TODO: load from file iso "environment"? - config: AggregatorConfig = get_config(args.environment) # Determine backend ids/urls aggregator_backends = get_backend_config().aggregator_backends diff --git a/tests/test_config.py b/tests/test_config.py index d0cfa06..fa6d421 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,18 +1,8 @@ -import os import textwrap -from pathlib import Path -from unittest import mock import pytest -from openeo_aggregator.config import ( - OPENEO_AGGREGATOR_CONFIG, - STREAM_CHUNK_SIZE_DEFAULT, - AggregatorBackendConfig, - AggregatorConfig, - ConfigException, - get_config, -) +from openeo_aggregator.config import AggregatorBackendConfig def _get_config_content(config_var_name: str = "config"): @@ -40,44 +30,3 @@ def test_config_aggregator_backends_empty(): def test_config_aggregator_backends(): config = AggregatorBackendConfig(aggregator_backends={"b1": "https://b1.test"}) assert config.aggregator_backends == {"b1": "https://b1.test"} - - -@pytest.mark.parametrize("config_var_name", ["aggregator_config", "config"]) -def test_config_from_py_file(tmp_path, config_var_name): - path = tmp_path / "aggregator-conf.py" - path.write_text(_get_config_content(config_var_name=config_var_name)) - config = AggregatorConfig.from_py_file(path) - assert config.config_source == str(path) - assert config.test_dummy == "bob" - - -def test_config_from_py_file_wrong_config_var_name(tmp_path): - path = tmp_path / "aggregator-conf.py" - path.write_text(_get_config_content(config_var_name="meh")) - with pytest.raises(ConfigException, match="No 'config' variable defined in config file"): - AggregatorConfig.from_py_file(path) - - -def test_get_config_default_no_env(): - assert OPENEO_AGGREGATOR_CONFIG not in os.environ - config = get_config() - assert config.config_source.endswith("/conf/aggregator.dummy.py") - - -@pytest.mark.parametrize("convertor", [str, Path]) -def test_get_config_py_file_path(tmp_path, convertor): - config_path = tmp_path / "aggregator-conf.py" - config_path.write_text(_get_config_content()) - config = get_config(convertor(config_path)) - assert config.config_source == str(config_path) - assert config.test_dummy == "bob" - - -def test_get_config_env_py_file(tmp_path): - path = tmp_path / "aggregator-conf.py" - path.write_text(_get_config_content()) - - with mock.patch.dict(os.environ, {OPENEO_AGGREGATOR_CONFIG: str(path)}): - config = get_config() - assert config.config_source == str(path) - assert config.test_dummy == "bob"