Issue #112 Add AggregatorBackendConfig.aggregator_backends

and deprecate AggregatorConfig.aggregator_backends

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 
 The format is roughly based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [Unreleased]
+
+- Add `AggregatorBackendConfig.aggregator_backends` and deprecate `AggregatorConfig.aggregator_backends` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112))
+
 ## [0.22.0]
 
 - Eliminate unused `AggregatorConfig.configured_oidc_providers` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112))

conf/aggregator.dev.py

@@ -44,13 +44,6 @@
 
 aggregator_config = AggregatorConfig(
     config_source=__file__,
-    aggregator_backends={
-        "vito": "https://openeo-dev.vito.be/openeo/1.1/",
-        "eodc": "https://openeo-dev.eodc.eu/openeo/1.1.0/",
-        "creo": "https://openeo-staging.creo.vito.be/openeo/1.1",
-        # Sentinel Hub OpenEO by Sinergise
-        "sentinelhub": "https://openeo.sentinel-hub.com/production/",
-    },
     partitioned_job_tracking={
         "zk_hosts": ZK_HOSTS,
     },
@@ -82,4 +75,11 @@
     capabilities_title="openEO Platform (dev)",
     capabilities_description="openEO Platform, provided through openEO Aggregator Driver (development instance).",
     oidc_providers=oidc_providers,
+    aggregator_backends={
+        "vito": "https://openeo-dev.vito.be/openeo/1.1/",
+        "eodc": "https://openeo-dev.eodc.eu/openeo/1.1.0/",
+        "creo": "https://openeo-staging.creo.vito.be/openeo/1.1",
+        # Sentinel Hub OpenEO by Sinergise
+        "sentinelhub": "https://openeo.sentinel-hub.com/production/",
+    },
 )

conf/aggregator.dummy.py

@@ -7,9 +7,6 @@
 
 aggregator_config = AggregatorConfig(
     config_source=__file__,
-    aggregator_backends={
-        "dummy": "https://openeo.example/openeo/1.1/",
-    },
     zookeeper_prefix="/openeo/aggregator/dummy/",
 )
 
@@ -18,6 +15,9 @@
     id="aggregator-dummy",
     capabilities_title="openEO Aggregator Dummy",
     capabilities_description="openEO Aggregator Dummy instance.",
+    aggregator_backends={
+        "dummy": "https://openeo.example/openeo/1.1/",
+    },
     oidc_providers=[
         OidcProvider(
             id="egi",

conf/aggregator.prod.py

@@ -37,12 +37,6 @@
 
 aggregator_config = AggregatorConfig(
     config_source=__file__,
-    aggregator_backends={
-        "vito": "https://openeo.vito.be/openeo/1.1/",
-        "eodc": "https://openeo.eodc.eu/openeo/1.1.0/",
-        # Sentinel Hub OpenEO by Sinergise
-        "sentinelhub": "https://openeo.sentinel-hub.com/production/",
-    },
     partitioned_job_tracking={
         "zk_hosts": ZK_HOSTS,
     },
@@ -75,4 +69,10 @@
     capabilities_title="openEO Platform",
     capabilities_description="openEO Platform, provided through openEO Aggregator Driver.",
     oidc_providers=oidc_providers,
+    aggregator_backends={
+        "vito": "https://openeo.vito.be/openeo/1.1/",
+        "eodc": "https://openeo.eodc.eu/openeo/1.1.0/",
+        # Sentinel Hub OpenEO by Sinergise
+        "sentinelhub": "https://openeo.sentinel-hub.com/production/",
+    },
 )

conf/backend_config.py

@@ -8,4 +8,5 @@
     capabilities_title="openEO Platform",
     capabilities_description="openEO Platform, provided through openEO Aggregator Driver",
     enable_basic_auth=False,
+    aggregator_backends={},
 )

src/openeo_aggregator/app.py

@@ -45,7 +45,6 @@ def create_app(config: Any = None, auto_logging_setup: bool = True, flask_error_
     config: AggregatorConfig = get_config(config)
     _log.info(f"Using config: {config.config_source=!r}")
 
-    _log.info(f"Creating MultiBackendConnection with {config.aggregator_backends=!r}")
     backends = MultiBackendConnection.from_config(config)
 
     _log.info("Creating AggregatorBackendImplementation")

src/openeo_aggregator/background/prime_caches.py

@@ -97,7 +97,6 @@ def prime_caches(
         config: AggregatorConfig = get_config(config)
         _log.info(f"Using config: {config.get('config_source')=}")
 
-        _log.info(f"Creating AggregatorBackendImplementation with {config.aggregator_backends}")
         backends = MultiBackendConnection.from_config(config)
         backend_implementation = AggregatorBackendImplementation(backends=backends, config=config)
 

src/openeo_aggregator/config.py

@@ -2,7 +2,7 @@
 import os
 import re
 from pathlib import Path
-from typing import Callable, List, Optional, Union
+from typing import Callable, Dict, List, Optional, Union
 
 import attrs
 from openeo_driver.config import OpenEoBackendConfig
@@ -43,7 +43,7 @@ class AggregatorConfig(dict):
     config_source = dict_item()
 
     # Dictionary mapping backend id to backend url
-    aggregator_backends = dict_item()
+    aggregator_backends = dict_item()  # TODO #112 deprecated
 
     partitioned_job_tracking = dict_item(default=None)
     # TODO #112 Deprecated, use AggregatorBackendConfig.zookeeper_prefix instead
@@ -126,6 +126,9 @@ class AggregatorBackendConfig(OpenEoBackendConfig):
         packages=["openeo", "openeo_driver", "openeo_aggregator"],
     )
 
+    # TODO #112 temporary default to allow migration, but make this field mandatory (and require non-empty)
+    aggregator_backends: Dict[str, str] = attrs.Factory(dict)
+
     streaming_chunk_size: int = STREAM_CHUNK_SIZE_DEFAULT
 
     auth_entitlement_check: Union[bool, dict] = False

src/openeo_aggregator/connection.py

@@ -232,6 +232,7 @@ def __init__(
                 f"to avoid collision issues when used as prefix. Got: {list(backends.keys())}"
             )
         # TODO: backend_urls as dict does not have explicit order, while this is important.
+        _log.info(f"Creating MultiBackendConnection with {backends=!r}")
         self._backend_urls = backends
         self._configured_oidc_providers = configured_oidc_providers
 
@@ -247,11 +248,12 @@ def __init__(
 
     @staticmethod
     def from_config(config: AggregatorConfig) -> 'MultiBackendConnection':
+        backend_config = get_backend_config()
         return MultiBackendConnection(
-            backends=config.aggregator_backends,
-            configured_oidc_providers=get_backend_config().oidc_providers,
+            backends=backend_config.aggregator_backends or config.aggregator_backends,
+            configured_oidc_providers=backend_config.oidc_providers,
             memoizer=memoizer_from_config(config, namespace="mbcon"),
-            connections_cache_ttl=get_backend_config().connections_cache_ttl,
+            connections_cache_ttl=backend_config.connections_cache_ttl,
         )
 
     def _get_connections(self, skip_failures=False) -> Iterator[BackendConnection]:

src/openeo_aggregator/metadata/validator.py

@@ -4,7 +4,7 @@
 
 import requests
 
-from openeo_aggregator.config import AggregatorConfig, get_config
+from openeo_aggregator.config import AggregatorConfig, get_backend_config, get_config
 from openeo_aggregator.metadata.merging import (
     ProcessMetadataMerger,
     merge_collection_metadata,
@@ -46,16 +46,16 @@ def main():
     _log.info(f"{args=}")
 
     # Load config
+    # TODO: load from file iso "environment"?
     config: AggregatorConfig = get_config(args.environment)
 
     # Determine backend ids/urls
-    backend_ids = args.backends or list(config.aggregator_backends.keys())
+    aggregator_backends = get_backend_config() or config.aggregator_backends
+    backend_ids = args.backends or list(aggregator_backends.keys())
     try:
-        backend_urls = [config.aggregator_backends[b] for b in backend_ids]
+        backend_urls = [aggregator_backends[b] for b in backend_ids]
     except KeyError:
-        raise ValueError(
-            f"Invalid backend ids {backend_ids}, should be subset of {list(config.aggregator_backends.keys())}."
-        )
+        raise ValueError(f"Invalid backend ids {backend_ids}, should be subset of {list(aggregator_backends.keys())}.")
     print("\nRunning metadata merging checks against backend urls:")
     print("\n".join(f"- {bid}: {url}" for bid, url in zip(backend_ids, backend_urls)))
     print("\n")

src/openeo_aggregator/testing.py

@@ -228,6 +228,12 @@ def capabilities(
 
         return capabilities
 
+    def credentials_oidc(self, id="egi", issuer="https://egi.test", title="EGI", extra: Optional[List[dict]] = None):
+        providers = [{"id": id, "issuer": issuer, "title": title}]
+        if extra:
+            providers += extra
+        return {"providers": providers}
+
     def collection(self, id="S2", *, license="proprietary") -> dict:
         """Build collection metadata"""
         return {

tests/backend_config.py

@@ -13,6 +13,10 @@
         OidcProvider(id="y-agg", issuer="https://y.test", title="Y (agg)"),
         OidcProvider(id="z-agg", issuer="https://z.test", title="Z (agg)"),
     ],
+    aggregator_backends={
+        "b1": "https://b1.test/v1",
+        "b2": "https://b2.test/v1",
+    },
     connections_cache_ttl=1.0,
     zookeeper_prefix="/o-a/",
 )

tests/background/test_prime_caches.py

@@ -23,10 +23,6 @@
 @pytest.fixture
 def config(backend1, backend2, backend1_id, backend2_id, zk_client) -> AggregatorConfig:
     conf = AggregatorConfig()
-    conf.aggregator_backends = {
-        backend1_id: backend1,
-        backend2_id: backend2,
-    }
     conf.memoizer = {
         "type": "zookeeper",
         "config": {
@@ -127,10 +123,6 @@ def test_prime_caches_main_basic(backend1, backend2, upstream_request_mocks, tmp
 
     # Construct config file
     config = AggregatorConfig()
-    config.aggregator_backends = {
-        backend1_id: backend1,
-        backend2_id: backend2,
-    }
     config_file = tmp_path / "conf.py"
     _build_config_file(config, config_file)
 
@@ -143,10 +135,6 @@ def test_prime_caches_main_logging(backend1, backend2, tmp_path, backend1_id, ba
     """Run main in subprocess (so no request mocks, and probably a lot of failures) to see if logging setup works."""
 
     config = AggregatorConfig()
-    config.aggregator_backends = {
-        backend1_id: backend1,
-        backend2_id: backend2,
-    }
     config_file = tmp_path / "conf.py"
     _build_config_file(config, config_file)
 

tests/conftest.py

@@ -49,12 +49,7 @@ def backend1(requests_mock, mbldr) -> str:
     domain = "https://b1.test/v1"
     # TODO: how to work with different API versions?
     requests_mock.get(domain + "/", json=mbldr.capabilities())
-    requests_mock.get(
-        domain + "/credentials/oidc",
-        json={
-            "providers": [{"id": "egi", "issuer": "https://egi.test", "title": "EGI"}]
-        },
-    )
+    requests_mock.get(domain + "/credentials/oidc", json=mbldr.credentials_oidc())
     requests_mock.get(domain + "/processes", json=mbldr.processes(*_DEFAULT_PROCESSES))
     return domain
 
@@ -63,12 +58,7 @@ def backend1(requests_mock, mbldr) -> str:
 def backend2(requests_mock, mbldr) -> str:
     domain = "https://b2.test/v1"
     requests_mock.get(domain + "/", json=mbldr.capabilities())
-    requests_mock.get(
-        domain + "/credentials/oidc",
-        json={
-            "providers": [{"id": "egi", "issuer": "https://egi.test", "title": "EGI"}]
-        },
-    )
+    requests_mock.get(domain + "/credentials/oidc", json=mbldr.credentials_oidc())
     requests_mock.get(domain + "/processes", json=mbldr.processes(*_DEFAULT_PROCESSES))
     return domain
 
@@ -128,10 +118,6 @@ def config(
 ) -> AggregatorConfig:
     """Config for most tests with two backends."""
     conf = base_config
-    conf.aggregator_backends = {
-        backend1_id: backend1,
-        backend2_id: backend2,
-    }
     return conf
 
 

tests/test_config.py

@@ -8,6 +8,7 @@
 from openeo_aggregator.config import (
     OPENEO_AGGREGATOR_CONFIG,
     STREAM_CHUNK_SIZE_DEFAULT,
+    AggregatorBackendConfig,
     AggregatorConfig,
     ConfigException,
     get_config,
@@ -20,24 +21,22 @@ def _get_config_content(config_var_name: str = "config"):
         from openeo_aggregator.config import AggregatorConfig
         {config_var_name} = AggregatorConfig(
             config_source=__file__,
-            aggregator_backends={{"b1": "https://b1.test"}},
             test_dummy="bob",
         )
         """
     )
 
 
+@pytest.mark.xfail(
+    reason="TODO #112: `aggregator_backends` should be mandatory, but to allow migration, it can currently be omitted."
+)
 def test_config_defaults():
-    config = AggregatorConfig()
-    with pytest.raises(KeyError):
-        _ = config.aggregator_backends
-    assert config.test_dummy == "alice"
+    with pytest.raises(TypeError, match="missing.*required.*aggregator_backends"):
+        _ = AggregatorBackendConfig()
 
 
 def test_config_aggregator_backends():
-    config = AggregatorConfig(
-        aggregator_backends={"b1": "https://b1.test"}
-    )
+    config = AggregatorBackendConfig(aggregator_backends={"b1": "https://b1.test"})
     assert config.aggregator_backends == {"b1": "https://b1.test"}
 
 
@@ -47,7 +46,6 @@ def test_config_from_py_file(tmp_path, config_var_name):
     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.aggregator_backends == {"b1": "https://b1.test"}
     assert config.test_dummy == "bob"
 
 
@@ -70,7 +68,6 @@ def test_get_config_py_file_path(tmp_path, convertor):
     config_path.write_text(_get_config_content())
     config = get_config(convertor(config_path))
     assert config.config_source == str(config_path)
-    assert config.aggregator_backends == {"b1": "https://b1.test"}
     assert config.test_dummy == "bob"
 
 
@@ -81,5 +78,4 @@ def test_get_config_env_py_file(tmp_path):
     with mock.patch.dict(os.environ, {OPENEO_AGGREGATOR_CONFIG: str(path)}):
         config = get_config()
     assert config.config_source == str(path)
-    assert config.aggregator_backends == {"b1": "https://b1.test"}
     assert config.test_dummy == "bob"