From 4b2f80fde89009d6225a1e83d039808d1dd2243c Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Thu, 8 Feb 2024 16:11:46 +0100 Subject: [PATCH] Issue #112 move collection_whitelist to AggregatorBackendConfig currently unused, so no migration path necessary related to eu-cdse/openeo-cdse-infra#54 --- CHANGELOG.md | 4 +++ src/openeo_aggregator/about.py | 2 +- src/openeo_aggregator/backend.py | 6 ++-- src/openeo_aggregator/config.py | 9 +++--- tests/test_views.py | 47 ++++++++++++++++---------------- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38efe27..69e407c 100644 --- a/CHANGELOG.md +++ b/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/). +## [0.18.4] + +- Move `collection_whitelist` config to `AggregatorBackendConfig` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112)) + ## [0.18.3] - Move `auth_entitlement_check` config to `AggregatorBackendConfig` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112)) diff --git a/src/openeo_aggregator/about.py b/src/openeo_aggregator/about.py index 42f4253..948e926 100644 --- a/src/openeo_aggregator/about.py +++ b/src/openeo_aggregator/about.py @@ -2,7 +2,7 @@ import sys from typing import Optional -__version__ = "0.18.3a1" +__version__ = "0.18.4a1" def log_version_info(logger: Optional[logging.Logger] = None): diff --git a/src/openeo_aggregator/backend.py b/src/openeo_aggregator/backend.py index 2b9f32d..c0bf685 100644 --- a/src/openeo_aggregator/backend.py +++ b/src/openeo_aggregator/backend.py @@ -150,7 +150,6 @@ def __init__(self, backends: MultiBackendConnection, config: AggregatorConfig): self.backends = backends self._memoizer = memoizer_from_config(config=config, namespace="CollectionCatalog") self.backends.on_connections_change.add(self._memoizer.invalidate) - self._collection_whitelist: Optional[List[str]] = config.collection_whitelist def get_all_metadata(self) -> List[dict]: metadata, internal = self._get_all_metadata_cached() @@ -168,6 +167,7 @@ def _get_all_metadata(self) -> Tuple[List[dict], _InternalCollectionMetadata]: """ # Group collection metadata by hierarchically: collection id -> backend id -> metadata grouped = defaultdict(dict) + collection_whitelist = get_backend_config().collection_whitelist with TimingLogger(title="Collect collection metadata from all backends", logger=_log): for con in self.backends: try: @@ -180,8 +180,8 @@ def _get_all_metadata(self) -> Tuple[List[dict], _InternalCollectionMetadata]: for collection_metadata in backend_collections: if "id" in collection_metadata: collection_id = collection_metadata["id"] - if self._collection_whitelist: - if collection_id not in self._collection_whitelist: + if collection_whitelist: + if collection_id not in collection_whitelist: _log.debug(f"Skipping non-whitelisted {collection_id=} from {con.id=}") continue else: diff --git a/src/openeo_aggregator/config.py b/src/openeo_aggregator/config.py index 5ceb490..f2427b0 100644 --- a/src/openeo_aggregator/config.py +++ b/src/openeo_aggregator/config.py @@ -1,7 +1,7 @@ import logging import os from pathlib import Path -from typing import Callable, List, Union +from typing import Callable, List, Optional, Union import attrs from openeo_driver.config import OpenEoBackendConfig @@ -57,9 +57,6 @@ class AggregatorConfig(dict): # TTL for connection caching. connections_cache_ttl = dict_item(default=5 * 60.0) - # List of collection ids to cover with the aggregator (when None: support union of all upstream collections) - collection_whitelist = dict_item(default=None) - # Just a config field for test purposes (while were stripping down this config class) test_dummy = dict_item(default="alice") @@ -138,6 +135,10 @@ class AggregatorBackendConfig(OpenEoBackendConfig): auth_entitlement_check: Union[bool, dict] = False + # List of collection ids to cover with the aggregator (when None: support union of all upstream collections) + collection_whitelist: Optional[List[str]] = None + + # Internal singleton _config_getter = ConfigGetter(expected_class=AggregatorBackendConfig) diff --git a/tests/test_views.py b/tests/test_views.py index 3846e3b..c060e0c 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -304,18 +304,18 @@ def test_collections_links(self, api100, requests_mock, backend1, backend2): } @pytest.mark.parametrize( - ["config_override", "expected"], + ["collection_whitelist", "expected"], [ - ({"collection_whitelist": None}, {"S1", "S2", "S3", "S4"}), - ({"collection_whitelist": []}, {"S1", "S2", "S3", "S4"}), - ({"collection_whitelist": ["S2"]}, {"S2"}), - ({"collection_whitelist": ["S4"]}, {"S4"}), - ({"collection_whitelist": ["S2", "S3"]}, {"S2", "S3"}), - ({"collection_whitelist": ["S2", "S999"]}, {"S2"}), - ({"collection_whitelist": ["S999"]}, set()), + (None, {"S1", "S2", "S3", "S4"}), + ([], {"S1", "S2", "S3", "S4"}), + (["S2"], {"S2"}), + (["S4"], {"S4"}), + (["S2", "S3"], {"S2", "S3"}), + (["S2", "S999"], {"S2"}), + (["S999"], set()), ], ) - def test_collections_whitelist(self, api100, requests_mock, backend1, backend2, expected): + def test_collections_whitelist(self, api100, requests_mock, backend1, backend2, collection_whitelist, expected): requests_mock.get(backend1 + "/collections", json={"collections": [{"id": "S1"}, {"id": "S2"}, {"id": "S3"}]}) for cid in ["S1", "S2", "S3"]: requests_mock.get(backend1 + f"/collections/{cid}", json={"id": cid, "title": f"b1 {cid}"}) @@ -323,23 +323,24 @@ def test_collections_whitelist(self, api100, requests_mock, backend1, backend2, for cid in ["S3", "S4"]: requests_mock.get(backend2 + f"/collections/{cid}", json={"id": cid, "title": f"b2 {cid}"}) - res = api100.get("/collections").assert_status_code(200).json - assert set(c["id"] for c in res["collections"]) == expected + with config_overrides(collection_whitelist=collection_whitelist): + res = api100.get("/collections").assert_status_code(200).json + assert set(c["id"] for c in res["collections"]) == expected - res = api100.get("/collections/S2") - if "S2" in expected: - assert res.assert_status_code(200).json == DictSubSet({"id": "S2", "title": "b1 S2"}) - else: - res.assert_error(404, "CollectionNotFound") + res = api100.get("/collections/S2") + if "S2" in expected: + assert res.assert_status_code(200).json == DictSubSet({"id": "S2", "title": "b1 S2"}) + else: + res.assert_error(404, "CollectionNotFound") - res = api100.get("/collections/S3") - if "S3" in expected: - assert res.assert_status_code(200).json == DictSubSet({"id": "S3", "title": "b1 S3"}) - else: - res.assert_error(404, "CollectionNotFound") + res = api100.get("/collections/S3") + if "S3" in expected: + assert res.assert_status_code(200).json == DictSubSet({"id": "S3", "title": "b1 S3"}) + else: + res.assert_error(404, "CollectionNotFound") - res = api100.get("/collections/S999") - res.assert_error(404, "CollectionNotFound") + res = api100.get("/collections/S999") + res.assert_error(404, "CollectionNotFound") class TestAuthentication: