#85 Code review: small changes to make things more readable

src/openeo_aggregator/backend.py

@@ -675,21 +675,13 @@ def _get_connection_and_backend_service_id(
         con = self._backends.get_connection(backend_id)
         return con, backend_service_id
 
-    def get_supported_backend_ids(self) -> List[str]:
+    def get_supporting_backend_ids(self) -> List[str]:
         """Get a list containing IDs of the backends that support secondary services."""
 
         # Try the cache first, but its is cached inside the result of get_service_types().
         # Getting the service types will also execute querying the capabilities for
         # which backends have secondary services.
-        return self._get_service_types_cached()["supported_backend_ids"]
-
-    def _find_ids_supported_backends(self) -> List[str]:
-        """Query the backends capabilities to find the ones that support secondary services."""
-        return [
-            con.id
-            for con in self._backends
-            if con.capabilities().supports_endpoint("/service_types")
-        ]
+        return self._get_service_types_cached()["supporting_backend_ids"]
 
     def service_types(self) -> dict:
         """https://openeo.org/documentation/1.0/developers/api/reference.html#operation/list-service-types"""
@@ -699,7 +691,7 @@ def service_types(self) -> dict:
 
     def _get_service_types_cached(self):
         return self._memoizer.get_or_call(
-            key=("service_types"), callback=self._get_service_types
+            key="service_types", callback=self._get_service_types
         )
 
     def _find_backend_id_for_service_type(self, service_type: str) -> str:
@@ -714,17 +706,15 @@ def _get_service_types(self) -> Dict:
         """Returns a dict with cacheable data about the supported backends and service types.
 
         :returns:
-            dict with the following fixed set of keys:
-            1. supported_backend_ids -> contains a list
-            2. service_type -> contains a dict
-
             It is a dict in order to keep it simple to cache this information.
+            The dict contains the following fixed set of keys:
 
-            1. "supported_backend_ids" maps to a list containing the backend IDs for
-                backends that support secondary services.
-                An aggregators may have backends that do not support secondary services.
+            supporting_backend_ids:
+                - Maps to a list of backend IDs for backends that support secondary services.
+                - An aggregator may have backends that do not support secondary services.
 
-            2. "service_types" maps to a dict with two items:
+            service_types
+                maps to a dict with two items:
                 - backend_id:
                     which backend provides that service type
                 - service_type:
@@ -734,7 +724,7 @@ def _get_service_types(self) -> Dict:
             For example:
 
             {
-                "supported_backend_ids": ["b1", "b2"]
+                "supporting_backend_ids": ["b1", "b2"]
                 "service_types": {
                     "WMTS": {
                         "backend_id": "b1",
@@ -771,15 +761,15 @@ def _get_service_types(self) -> Dict:
         # Along with the service types we query which backends support secondary services
         # so we can cache that information.
         # Some backends don not have the GET /service_types endpoint.
-        supported_backend_ids = self._find_ids_supported_backends()
+        supporting_backend_ids = [
+            con.id
+            for con in self._backends
+            if con.capabilities().supports_endpoint("/service_types")
+        ]
         service_types = {}
-        result = {
-            "supported_backend_ids": supported_backend_ids,
-            "service_types": service_types,
-        }
 
         # Collect all service types from the backends.
-        for backend_id in supported_backend_ids:
+        for backend_id in supporting_backend_ids:
             con = self._backends.get_connection(backend_id)
             try:
                 types_to_add = con.get("/service_types").json()
@@ -798,13 +788,16 @@ def _get_service_types(self) -> Dict:
                         f'Conflicting secondary service types: "{name}" is present in more than one backend, ' +
                         f'already found in backend: {conflicting_backend}'
                     )
-        return result
+        return {
+            "supporting_backend_ids": supporting_backend_ids,
+            "service_types": service_types,
+        }
 
     def list_services(self, user_id: str) -> List[ServiceMetadata]:
         """https://openeo.org/documentation/1.0/developers/api/reference.html#operation/list-services"""
         services = []
 
-        for backend_id in self.get_supported_backend_ids():
+        for backend_id in self.get_supporting_backend_ids():
             con = self._backends.get_connection(backend_id)
             with con.authenticated_from_request(
                 request=flask.request, user=User(user_id)

tests/test_backend.py

@@ -157,16 +157,16 @@ class TestAggregatorSecondaryServices:
         }
     }
 
-    def test_get_supported_backend_ids_none_supported(
+    def test_get_supporting_backend_ids_none_supported(
         self, multi_backend_connection, config, catalog
     ):
         processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config)
         implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing, config=config)
 
-        actual_supported_backends = implementation.get_supported_backend_ids()
+        actual_supported_backends = implementation.get_supporting_backend_ids()
         assert actual_supported_backends == []
 
-    def test_get_supported_backend_ids_all_supported(
+    def test_get_supporting_backend_ids_all_supported(
         self, multi_backend_connection, config, catalog, backend1, backend2, requests_mock,
         json_capabilities_with_service_types_supported
     ):
@@ -175,10 +175,10 @@ def test_get_supported_backend_ids_all_supported(
         processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config)
         implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing, config=config)
 
-        actual_supported_backends = implementation.get_supported_backend_ids()
+        actual_supported_backends = implementation.get_supporting_backend_ids()
         assert actual_supported_backends == ["b1", "b2"]
 
-    def test_get_supported_backend_ids_only_one_supported(
+    def test_get_supporting_backend_ids_only_one_supported(
         self, multi_backend_connection, config, catalog, backend1, backend2, requests_mock,
         json_capabilities_with_service_types_supported, json_capabilities_no_endpoints
     ):
@@ -187,7 +187,7 @@ def test_get_supported_backend_ids_only_one_supported(
         processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config)
         implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing, config=config)
 
-        actual_supported_backends = implementation.get_supported_backend_ids()
+        actual_supported_backends = implementation.get_supporting_backend_ids()
         assert actual_supported_backends == ["b2"]
 
     def test_service_types_simple(
@@ -265,14 +265,20 @@ def test_service_types_skips_unsupported_backend(
             when the aggregator fulfills a request to list the service types,
             then the aggregator skips the unsupported backend.
         """
-        single_service_type = self.SERVICE_TYPES_ONLT_WMTS
-        mock_b1_service_types = requests_mock.get(backend1 + "/service_types", status_code=500) # Not supported
-        mock_b2_service_types = requests_mock.get(backend2 + "/service_types", json=single_service_type)
         # We are testing that the aggregator checks if the backend supports GET /service_types, so we have to mock that up too.
         # We want to verify that the capabilities are actually queried.
         mock_b1_capabilities = requests_mock.get(backend1 + "/", json=json_capabilities_no_endpoints)
         mock_b2_capabilities = requests_mock.get(backend2 + "/", json=json_capabilities_with_service_types_supported)
 
+        single_service_type = self.SERVICE_TYPES_ONLT_WMTS
+        # Backend 1 does support secondary services
+        mock_b1_service_types = requests_mock.get(
+            backend1 + "/service_types", status_code=500
+        )
+        mock_b2_service_types = requests_mock.get(
+            backend2 + "/service_types", json=single_service_type
+        )
+
         processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config)
         implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing, config=config)
 

tests/test_views.py

@@ -1521,7 +1521,7 @@ def test_list_services_no_supporting_backends(
         caplog.set_level(logging.ERROR)
         api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN)
 
-        # No backends supported ==> no mock for capabilities at "GET /"
+        # No backends supported ==> Rely on default mock in backend1 for capabilities at "GET /"
         # But the backend's /services endpoint should not be called in this scenario,
         # so make it a fail in a loud way if the aggregator *does* call it.
         mock_backend_services = requests_mock.get(