Issue #112 WIP

setup.py

@@ -33,6 +33,7 @@
         "gunicorn~=20.0",
         "python-json-logger>=2.0.0",
         "kazoo~=2.8.0",
+        "attrs",
     ],
     tests_require=tests_require,
     extras_require={

src/openeo_aggregator/app.py

@@ -70,6 +70,7 @@ def create_app(config: Any = None, auto_logging_setup: bool = True) -> flask.Fla
         packages=["openeo", "openeo_driver", "openeo_aggregator"],
     )
     app.config.from_mapping(
+        # TODO: move to new config system here
         OPENEO_TITLE="openEO Platform",
         OPENEO_DESCRIPTION="openEO Platform, provided through openEO Aggregator Driver",
         OPENEO_BACKEND_VERSION=openeo_aggregator.about.__version__,

src/openeo_aggregator/config.py

@@ -2,10 +2,11 @@
 import os
 import re
 from pathlib import Path
-from typing import Any, List, Union
+from typing import Any, Dict, List, Optional, Union
 
+import attrs
+from openeo_driver.config import OpenEoBackendConfig
 from openeo_driver.users.oidc import OidcProvider
-from openeo_driver.utils import dict_item
 
 _log = logging.getLogger(__name__)
 
@@ -27,31 +28,36 @@ class ConfigException(ValueError):
     pass
 
 
-# TODO #112 subclass from OpenEoBackendConfig
-class AggregatorConfig(dict):
+@attrs.frozen(
+    # Note: `kw_only=True` enforces "kwargs" based construction (which is good for readability/maintainability)
+    # and allows defining mandatory fields (fields without default) after optional fields.
+    kw_only=True
+)
+class AggregatorConfig(OpenEoBackendConfig):
     """
     Simple dictionary based configuration for aggregator backend
     """
-    config_source = dict_item()
+
+    config_source: str = "n/a"
 
     # Dictionary mapping backend id to backend url
-    aggregator_backends = dict_item()
+    aggregator_backends: Dict[str, str] = attrs.Factory(dict)
 
-    flask_error_handling = dict_item(default=True)
-    streaming_chunk_size = dict_item(default=STREAM_CHUNK_SIZE_DEFAULT)
+    flask_error_handling: bool = True
+    streaming_chunk_size: int = STREAM_CHUNK_SIZE_DEFAULT
 
     # TODO: add validation/normalization to make sure we have a real list of OidcProvider objects?
-    configured_oidc_providers: List[OidcProvider] = dict_item(default=[])
-    auth_entitlement_check: Union[bool, dict] = dict_item()
+    configured_oidc_providers: List[OidcProvider] = attrs.Factory(list)
+    auth_entitlement_check: Union[bool, dict] = attrs.Factory(dict)
 
-    partitioned_job_tracking = dict_item(default=None)
-    zookeeper_prefix = dict_item(default="/openeo-aggregator/")
+    partitioned_job_tracking: Optional[dict] = None
+    zookeeper_prefix: str = "/openeo-aggregator/"
 
     # See `memoizer_from_config` for details.
-    memoizer = dict_item(default={"type": "dict"})
+    memoizer: dict = attrs.Factory(lambda: {"type": "dict"})
 
     # TTL for connection caching.
-    connections_cache_ttl = dict_item(default=5 * 60.0)
+    connections_cache_ttl: float = 5 * 60.0
 
     @staticmethod
     def from_py_file(path: Union[str, Path]) -> 'AggregatorConfig':

tests/conftest.py

@@ -101,52 +101,59 @@ def connections_cache_ttl() -> float:
 
 
 @pytest.fixture
-def base_config(
-    configured_oidc_providers, zk_client, memoizer_config, connections_cache_ttl
-) -> AggregatorConfig:
-    """Base config for tests (without any configured backends)."""
-    conf = AggregatorConfig()
-    # conf.flask_error_handling = False  # Temporary disable flask error handlers to simplify debugging (better stack traces).
-
-    conf.configured_oidc_providers = configured_oidc_providers
-    # Disable OIDC/EGI entitlement check by default.
-    conf.auth_entitlement_check = False
+def aggregator_backends(backend1, backend2) -> dict:
+    return {
+        "b1": backend1,
+        "b2": backend2,
+    }
 
-    conf.memoizer = memoizer_config
-    conf.connections_cache_ttl = connections_cache_ttl
 
-    conf.zookeeper_prefix = "/o-a/"
-    conf.partitioned_job_tracking = {
-        "zk_client": zk_client,
-    }
-    return conf
+@pytest.fixture
+def partitioned_job_tracking(zk_client) -> dict:
+    return {"zk_client": zk_client}
 
 
 @pytest.fixture
-def backend1_id() -> str:
-    """Id of first upstream backend. As a fixture to allow per-test override"""
-    return "b1"
+def auth_entitlement_check() -> bool:
+    # Disable OIDC/EGI entitlement check by default.
+    return False
 
 
 @pytest.fixture
-def backend2_id() -> str:
-    """Id of second upstream backend. As a fixture to allow per-test override"""
-    return "b2"
+def config_overrides() -> dict:
+    """Fixture for configuration overrides through parameterization"""
+    return {}
 
 
 @pytest.fixture
 def config(
-    base_config, backend1, backend2, backend1_id, backend2_id
+    aggregator_backends,
+    configured_oidc_providers,
+    zk_client,
+    memoizer_config,
+    connections_cache_ttl,
+    partitioned_job_tracking,
+    auth_entitlement_check,
+    config_overrides,
 ) -> AggregatorConfig:
-    """Config for most tests with two backends."""
-    conf = base_config
-    conf.aggregator_backends = {
-        backend1_id: backend1,
-        backend2_id: backend2,
-    }
+    """Base config for tests (without any configured backends)."""
+    conf = AggregatorConfig(
+        aggregator_backends=aggregator_backends,
+        # flask_error_handling=False, # Temporary disable flask error handlers to simplify debugging (better stack traces).
+        configured_oidc_providers=configured_oidc_providers,
+        # Disable OIDC/EGI entitlement check by default.
+        auth_entitlement_check=auth_entitlement_check,
+        memoizer=memoizer_config,
+        connections_cache_ttl=connections_cache_ttl,
+        zookeeper_prefix="/o-a/",
+        partitioned_job_tracking=partitioned_job_tracking,
+        **config_overrides,
+    )
+
     return conf
 
 
+
 @pytest.fixture
 def multi_backend_connection(config) -> MultiBackendConnection:
     return MultiBackendConnection.from_config(config)
@@ -181,11 +188,6 @@ def api100(flask_app: flask.Flask) -> ApiTester:
     return get_api100(flask_app)
 
 
-@pytest.fixture
-def api100_with_entitlement_check(config: AggregatorConfig) -> ApiTester:
-    config.auth_entitlement_check = {"oidc_issuer_whitelist": {"https://egi.test", "https://egi.test/oidc"}}
-    return get_api100(get_flask_app(config))
-
 
 def assert_dict_subset(d1: dict, d2: dict):
     """Check whether dictionary `d1` is a subset of `d2`"""

tests/test_app.py

@@ -17,7 +17,6 @@ def test_create_app(config: AggregatorConfig):
     ({"zk_client": "dummy"}, True),
 ])
 def test_create_app_no_partitioned_job_tracking(config: AggregatorConfig, partitioned_job_tracking, expected):
-    config.partitioned_job_tracking = partitioned_job_tracking
     api100 = get_api100(get_flask_app(config))
     res = api100.get("/").assert_status_code(200).json
     assert res["_partitioned_job_tracking"] is expected

tests/test_caching.py

@@ -642,15 +642,20 @@ def test_corrupted_cache(self, zk_client, caplog):
         assert "corrupt data path '/test/count'" in caplog.text
 
     @clock_mock(0)
-    def test_from_config(self, config, zk_client):
-        config.memoizer = {
-            "type": "zookeeper",
-            "config": {
-                "zk_hosts": "zk1.test:2181,zk2.test:2181",
-                "default_ttl": 123,
-                "zk_timeout": 7.25,
+    @pytest.mark.parametrize(
+        "memoizer_config",
+        [
+            {
+                "type": "zookeeper",
+                "config": {
+                    "zk_hosts": "zk1.test:2181,zk2.test:2181",
+                    "default_ttl": 123,
+                    "zk_timeout": 7.25,
+                },
             }
-        }
+        ],
+    )
+    def test_from_config(self, config, zk_client, memoizer_config):
         with mock.patch.object(openeo_aggregator.caching, "KazooClient", return_value=zk_client) as KazooClient:
             zk_cache = memoizer_from_config(config, namespace="tezt")
 
@@ -699,48 +704,46 @@ def fun2_cached():
 class TestMemoizerFromConfig:
 
     def test_null_memoizer(self):
-        config = AggregatorConfig()
-        config.memoizer = {"type": "null"}
+        config = AggregatorConfig(memoizer={"type": "null"})
         memoizer = memoizer_from_config(config, namespace="test")
         assert isinstance(memoizer, NullMemoizer)
 
     def test_dict_memoizer(self):
-        config = AggregatorConfig()
-        config.memoizer = {"type": "dict", "config": {"default_ttl": 99}}
+        config = AggregatorConfig(memoizer={"type": "dict", "config": {"default_ttl": 99}})
         memoizer = memoizer_from_config(config, namespace="test")
         assert isinstance(memoizer, DictMemoizer)
         assert memoizer._default_ttl == 99
 
     def test_jsondict_memoizer(self):
-        config = AggregatorConfig()
-        config.memoizer = {"type": "jsondict", "config": {"default_ttl": 99}}
+        config = AggregatorConfig(memoizer={"type": "jsondict", "config": {"default_ttl": 99}})
         memoizer = memoizer_from_config(config, namespace="test")
         assert isinstance(memoizer, JsonDictMemoizer)
         assert memoizer._default_ttl == 99
 
     def test_zookeeper_memoizer(self):
-        config = AggregatorConfig()
-        config.memoizer = {
-            "type": "zookeeper",
-            "config": {"zk_hosts": "zk.test:2181", "default_ttl": 99, "zk_timeout": 88}
-        }
-        config.zookeeper_prefix = "/oea/test"
+        config = AggregatorConfig(
+            memoizer={"type": "zookeeper", "config": {"zk_hosts": "zk.test:2181", "default_ttl": 99, "zk_timeout": 88}},
+            zookeeper_prefix="/oea/test",
+        )
         memoizer = memoizer_from_config(config, namespace="test-ns")
         assert isinstance(memoizer, ZkMemoizer)
         assert memoizer._default_ttl == 99
         assert memoizer._prefix == "/oea/test/cache/test-ns"
         assert memoizer._zk_timeout == 88
 
     def test_chained_memoizer(self):
-        config = AggregatorConfig()
-        config.memoizer = {
-            "type": "chained",
-            "config": {"parts": [
-                {"type": "jsondict", "config": {"default_ttl": 99}},
-                {"type": "dict", "config": {"default_ttl": 333}},
-            ]}
-        }
-        config.zookeeper_prefix = "/oea/test"
+        config = AggregatorConfig(
+            memoizer={
+                "type": "chained",
+                "config": {
+                    "parts": [
+                        {"type": "jsondict", "config": {"default_ttl": 99}},
+                        {"type": "dict", "config": {"default_ttl": 333}},
+                    ]
+                },
+            },
+            zookeeper_prefix="/oea/test",
+        )
         memoizer = memoizer_from_config(config, namespace="test-ns")
         assert isinstance(memoizer, ChainedMemoizer)
         assert len(memoizer._memoizers) == 2

tests/test_config.py

@@ -24,8 +24,7 @@
 
 def test_config_defaults():
     config = AggregatorConfig()
-    with pytest.raises(KeyError):
-        _ = config.aggregator_backends
+    assert config.aggregator_backends == {}
     assert config.flask_error_handling is True
     assert config.streaming_chunk_size == STREAM_CHUNK_SIZE_DEFAULT