handles exceptions using warnings

Author: pcrespov

services/catalog/src/simcore_service_catalog/core/background_tasks.py

@@ -19,8 +19,10 @@
 from models_library.services import ServiceMetaDataPublished
 from models_library.services_types import ServiceKey, ServiceVersion
 from packaging.version import Version
+from pydantic import ValidationError
 from simcore_service_catalog.api.dependencies.director import get_director_api
 from simcore_service_catalog.services import manifest
+from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.ext.asyncio import AsyncEngine
 
 from ..db.repositories.groups import GroupsRepository
@@ -62,6 +64,7 @@ def _by_version(t: tuple[ServiceKey, ServiceVersion]) -> Version:
     sorted_services = sorted(service_keys, key=_by_version)
 
     for service_key, service_version in sorted_services:
+
         service_metadata: ServiceMetaDataPublished = services_in_registry[
             (service_key, service_version)
         ]
@@ -90,11 +93,15 @@ def _by_version(t: tuple[ServiceKey, ServiceVersion]) -> Version:
                 service_access_rights,
             )
 
-        except HTTPException as err:
-            # calls to director migh fail but this should not stop the background task from running.
+        except (HTTPException, ValidationError, SQLAlchemyError) as err:
+            # Resilient to single failures: errors in individual (service,key) should not affect the wholse serices
+            # and stop the background task from running.
             # SEE https://github.com/ITISFoundation/osparc-simcore/issues/6318
             _logger.warning(
-                "Skipping '%s:%s'. Reason: %s", service_key, service_version, err
+                "Skipping '%s:%s' due to %s",
+                service_key,
+                service_version,
+                err,
             )
 
 

services/catalog/src/simcore_service_catalog/services/access_rights.py

@@ -5,7 +5,7 @@
 import logging
 import operator
 from collections.abc import Callable
-from datetime import datetime, timezone
+from datetime import UTC, datetime
 from typing import Any, cast
 from urllib.parse import quote_plus
 
@@ -25,18 +25,14 @@
 
 _logger = logging.getLogger(__name__)
 
-_LEGACY_SERVICES_DATE: datetime = datetime(
-    year=2020, month=8, day=19, tzinfo=timezone.utc
-)
+_LEGACY_SERVICES_DATE: datetime = datetime(year=2020, month=8, day=19, tzinfo=UTC)
 
 
 def _is_frontend_service(service: ServiceMetaDataPublished) -> bool:
     return "/frontend/" in service.key
 
 
 async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bool:
-
-    # FIXME: MUST NOT raise
     # NOTE: https://github.com/ITISFoundation/osparc-simcore/pull/6003#discussion_r1658200909
     # get service build date
     client = get_director_api(app)
@@ -48,9 +44,6 @@ async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bo
     )
     if not data or "build_date" not in data:
         return True
-
-    _logger.debug("retrieved service extras are %s", data)
-
     service_build_data = arrow.get(data["build_date"]).datetime
     return bool(service_build_data < _LEGACY_SERVICES_DATE)
 
@@ -65,6 +58,12 @@ async def evaluate_default_policy(
         1. All services published in osparc prior 19.08.2020 will be visible to everyone (refered as 'old service').
         2. Services published after 19.08.2020 will be visible ONLY to his/her owner
         3. Front-end services are have execute-access to everyone
+
+
+    Raises:
+        HTTPException: from calls to director's rest API. Maps director errors into catalog's server error
+        SQLAlchemyError: from access to pg database
+        ValidationError: from pydantic model errors
     """
     db_engine: AsyncEngine = app.state.engine
 

services/catalog/tests/unit/with_dbs/test_core_background_task__sync.py

@@ -9,7 +9,8 @@
 from typing import Any
 
 import pytest
-from fastapi import FastAPI
+from fastapi import FastAPI, HTTPException, status
+from pytest_mock import MockerFixture
 from respx.router import MockRouter
 from simcore_postgres_database.models.services import services_meta_data
 from simcore_service_catalog.core.background_tasks import _run_sync_services
@@ -40,6 +41,7 @@ async def cleanup_service_meta_data_db_content(sqlalchemy_async_engine: AsyncEng
         await conn.execute(services_meta_data.delete())
 
 
+@pytest.mark.parametrize("director_fails", [False, True])
 async def test_registry_sync_task(
     background_tasks_setup_disabled: None,
     rabbitmq_and_rpc_setup_disabled: None,
@@ -49,10 +51,20 @@ async def test_registry_sync_task(
     app: FastAPI,
     services_repo: ServicesRepository,
     cleanup_service_meta_data_db_content: None,
+    mocker: MockerFixture,
+    director_fails: bool,
 ):
-
     assert app.state
 
+    if director_fails:
+        # Emulates issue https://github.com/ITISFoundation/osparc-simcore/issues/6318
+        mocker.patch(
+            "simcore_service_catalog.services.access_rights._is_old_service",
+            side_effect=HTTPException(
+                status_code=status.HTTP_404_NOT_FOUND, detail="fake director error"
+            ),
+        )
+
     service_key = expected_director_list_services[0]["key"]
     service_version = expected_director_list_services[0]["version"]
 
@@ -75,6 +87,10 @@ async def test_registry_sync_task(
         key=service_key,
         version=service_version,
     )
-    assert got_from_db
-    assert got_from_db.key == service_key
-    assert got_from_db.version == service_version
+
+    if director_fails:
+        assert not got_from_db
+    else:
+        assert got_from_db
+        assert got_from_db.key == service_key
+        assert got_from_db.version == service_version