@sanderegg review: rename director-api by director-client

pcrespov · pcrespov · commit 435f2867dab8 · 2025-04-10T18:16:46.000+02:00
diff --git a/services/catalog/src/simcore_service_catalog/api/_dependencies/director.py b/services/catalog/src/simcore_service_catalog/api/_dependencies/director.py
@@ -3,11 +3,11 @@
 from fastapi import Depends, FastAPI
 from servicelib.fastapi.dependencies import get_app
 
-from ...clients.director import DirectorApi
+from ...clients.director import DirectorClient
 
 
-def get_director_api(
+def get_director_client(
     app: Annotated[FastAPI, Depends(get_app)],
-) -> DirectorApi:
-    director: DirectorApi = app.state.director_api
+) -> DirectorClient:
+    director: DirectorClient = app.state.director_api
     return director
diff --git a/services/catalog/src/simcore_service_catalog/api/_dependencies/services.py b/services/catalog/src/simcore_service_catalog/api/_dependencies/services.py
@@ -12,12 +12,12 @@
 from pydantic import ValidationError
 from servicelib.fastapi.dependencies import get_app
 
-from ...clients.director import DirectorApi
+from ...clients.director import DirectorClient
 from ...core.settings import ApplicationSettings
 from ...repository.groups import GroupsRepository
 from ...repository.services import ServicesRepository
 from ...service import manifest
-from .director import get_director_api
+from .director import get_director_client
 from .repository import get_repository
 
 _logger = logging.getLogger(__name__)
@@ -86,7 +86,7 @@ async def check_service_read_access(
 async def get_service_from_manifest(
     service_key: ServiceKey,
     service_version: ServiceVersion,
-    director_client: Annotated[DirectorApi, Depends(get_director_api)],
+    director_client: Annotated[DirectorClient, Depends(get_director_client)],
 ) -> ServiceMetaDataPublished:
     """
     Retrieves service metadata from the docker registry via the director
diff --git a/services/catalog/src/simcore_service_catalog/api/rest/_services.py b/services/catalog/src/simcore_service_catalog/api/rest/_services.py
@@ -20,11 +20,11 @@
     LIST_SERVICES_CACHING_TTL,
     RESPONSE_MODEL_POLICY,
 )
-from ...clients.director import DirectorApi
+from ...clients.director import DirectorClient
 from ...models.services_db import ServiceAccessRightsAtDB, ServiceMetaDataDBGet
 from ...repository.groups import GroupsRepository
 from ...repository.services import ServicesRepository
-from .._dependencies.director import get_director_api
+from .._dependencies.director import get_director_client
 from .._dependencies.repository import get_repository
 from .._dependencies.services import get_service_from_manifest
 
@@ -86,7 +86,7 @@ async def list_services(
     request: Request,  # pylint:disable=unused-argument
     *,
     user_id: PositiveInt,
-    director_client: Annotated[DirectorApi, Depends(get_director_api)],
+    director_client: Annotated[DirectorClient, Depends(get_director_client)],
     groups_repository: Annotated[
         GroupsRepository, Depends(get_repository(GroupsRepository))
     ],
diff --git a/services/catalog/src/simcore_service_catalog/api/rest/_services_extras.py b/services/catalog/src/simcore_service_catalog/api/rest/_services_extras.py
@@ -4,9 +4,9 @@
 from models_library.api_schemas_directorv2.services import ServiceExtras
 from models_library.services import ServiceKey, ServiceVersion
 
-from ...clients.director import DirectorApi
+from ...clients.director import DirectorClient
 from ...service import services
-from .._dependencies.director import get_director_api
+from .._dependencies.director import get_director_client
 
 router = APIRouter()
 
@@ -15,7 +15,7 @@
 async def get_service_extras(
     service_key: ServiceKey,
     service_version: ServiceVersion,
-    director_client: Annotated[DirectorApi, Depends(get_director_api)],
+    director_client: Annotated[DirectorClient, Depends(get_director_client)],
 ) -> ServiceExtras:
 
     return await services.get_service_extras(
diff --git a/services/catalog/src/simcore_service_catalog/api/rest/_services_labels.py b/services/catalog/src/simcore_service_catalog/api/rest/_services_labels.py
@@ -3,8 +3,8 @@
 from fastapi import APIRouter, Depends
 from models_library.services import ServiceKey, ServiceVersion
 
-from ...clients.director import DirectorApi
-from .._dependencies.director import get_director_api
+from ...clients.director import DirectorClient
+from .._dependencies.director import get_director_client
 
 router = APIRouter()
 
@@ -13,6 +13,6 @@
 async def get_service_labels(
     service_key: ServiceKey,
     service_version: ServiceVersion,
-    director_client: Annotated[DirectorApi, Depends(get_director_api)],
+    director_client: Annotated[DirectorClient, Depends(get_director_client)],
 ) -> dict[str, Any]:
     return await director_client.get_service_labels(service_key, service_version)
diff --git a/services/catalog/src/simcore_service_catalog/api/rest/_services_resources.py b/services/catalog/src/simcore_service_catalog/api/rest/_services_resources.py
@@ -23,14 +23,14 @@
 from pydantic import TypeAdapter
 
 from ..._constants import RESPONSE_MODEL_POLICY, SIMCORE_SERVICE_SETTINGS_LABELS
-from ...clients.director import DirectorApi
+from ...clients.director import DirectorClient
 from ...repository.services import ServicesRepository
 from ...service.function_services import is_function_service
 from ...utils.service_resources import (
     merge_service_resources_with_user_specs,
     parse_generic_resource,
 )
-from .._dependencies.director import get_director_api
+from .._dependencies.director import get_director_client
 from .._dependencies.repository import get_repository
 from .._dependencies.services import get_default_service_resources
 from .._dependencies.user_groups import list_user_groups
@@ -128,7 +128,7 @@ def _resources_from_settings(
 
 
 async def _get_service_labels(
-    director_client: DirectorApi, key: ServiceKey, version: ServiceVersion
+    director_client: DirectorClient, key: ServiceKey, version: ServiceVersion
 ) -> dict[str, Any] | None:
     try:
         service_labels = await director_client.get_service_labels(key, version)
@@ -166,7 +166,7 @@ def _get_service_settings(
 async def get_service_resources(
     service_key: ServiceKey,
     service_version: ServiceVersion,
-    director_client: Annotated[DirectorApi, Depends(get_director_api)],
+    director_client: Annotated[DirectorClient, Depends(get_director_client)],
     default_service_resources: Annotated[
         ResourcesDict, Depends(get_default_service_resources)
     ],
diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/_services.py b/services/catalog/src/simcore_service_catalog/api/rpc/_services.py
@@ -27,7 +27,7 @@
 
 from ...repository.services import ServicesRepository
 from ...service import services
-from .._dependencies.director import get_director_api
+from .._dependencies.director import get_director_client
 
 _logger = logging.getLogger(__name__)
 
@@ -70,7 +70,7 @@ async def list_services_paginated(
 
     total_count, items = await services.list_latest_services(
         repo=ServicesRepository(app.state.engine),
-        director_api=get_director_api(app),
+        director_api=get_director_client(app),
         product_name=product_name,
         user_id=user_id,
         limit=limit,
@@ -113,7 +113,7 @@ async def get_service(
 
     service = await services.get_service(
         repo=ServicesRepository(app.state.engine),
-        director_api=get_director_api(app),
+        director_api=get_director_client(app),
         product_name=product_name,
         user_id=user_id,
         service_key=service_key,
@@ -150,7 +150,7 @@ async def update_service(
 
     service = await services.update_service(
         repo=ServicesRepository(app.state.engine),
-        director_api=get_director_api(app),
+        director_api=get_director_client(app),
         product_name=product_name,
         user_id=user_id,
         service_key=service_key,
diff --git a/services/catalog/src/simcore_service_catalog/clients/director.py b/services/catalog/src/simcore_service_catalog/clients/director.py
@@ -107,7 +107,7 @@ def _unenvelope_or_raise_error(
 
     @functools.wraps(request_func)
     async def request_wrapper(
-        zelf: "DirectorApi", path: str, *args, **kwargs
+        zelf: "DirectorClient", path: str, *args, **kwargs
     ) -> list[Any] | dict[str, Any]:
         normalized_path = path.lstrip("/")
         try:
@@ -126,7 +126,7 @@ async def request_wrapper(
     return request_wrapper
 
 
-class DirectorApi:
+class DirectorClient:
     """
     - wrapper around thin-client to simplify director's API
     - sets endspoint upon construction
@@ -292,7 +292,7 @@ async def get_service_extras(
 
 
 async def director_lifespan(app: FastAPI) -> AsyncIterator[State]:
-    client: DirectorApi | None = None
+    client: DirectorClient | None = None
     settings = app.state.settings.CATALOG_DIRECTOR
 
     assert isinstance(settings, DirectorSettings)  # nosec
@@ -302,7 +302,7 @@ async def director_lifespan(app: FastAPI) -> AsyncIterator[State]:
     ):
         async for attempt in AsyncRetrying(**_director_startup_retry_policy):
             with attempt:
-                client = DirectorApi(base_url=settings.base_url, app=app)
+                client = DirectorClient(base_url=settings.base_url, app=app)
                 if not await client.is_responsive():
                     with suppress(Exception):
                         await client.close()
diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py
@@ -22,7 +22,7 @@
 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.api._dependencies.director import get_director_client
 from simcore_service_catalog.service import manifest
 from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.ext.asyncio import AsyncEngine
@@ -114,7 +114,7 @@ async def _ensure_registry_and_database_are_synced(app: FastAPI) -> None:
 
     Notice that a services here refers to a 2-tuple (key, version)
     """
-    director_api = get_director_api(app)
+    director_api = get_director_client(app)
     services_in_manifest_map = await manifest.get_services_map(director_api)
 
     services_in_db: set[tuple[ServiceKey, ServiceVersion]] = (
diff --git a/services/catalog/src/simcore_service_catalog/service/access_rights.py b/services/catalog/src/simcore_service_catalog/service/access_rights.py
@@ -14,7 +14,7 @@
 from pydantic.types import PositiveInt
 from sqlalchemy.ext.asyncio import AsyncEngine
 
-from ..api._dependencies.director import get_director_api
+from ..api._dependencies.director import get_director_client
 from ..models.services_db import ServiceAccessRightsAtDB
 from ..repository.groups import GroupsRepository
 from ..repository.services import ServicesRepository
@@ -32,7 +32,7 @@ def _is_frontend_service(service: ServiceMetaDataPublished) -> bool:
 async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bool:
     # NOTE: https://github.com/ITISFoundation/osparc-simcore/pull/6003#discussion_r1658200909
     # get service build date
-    client = get_director_api(app)
+    client = get_director_client(app)
 
     data = await client.get_service_extras(service.key, service.version)
     if not data or data.service_build_details is None:
diff --git a/services/catalog/src/simcore_service_catalog/service/manifest.py b/services/catalog/src/simcore_service_catalog/service/manifest.py
@@ -35,7 +35,7 @@
 from servicelib.utils import limited_gather
 
 from .._constants import DIRECTOR_CACHING_TTL
-from ..clients.director import DirectorApi
+from ..clients.director import DirectorClient
 from .function_services import get_function_service, is_function_service
 
 _logger = logging.getLogger(__name__)
@@ -50,7 +50,7 @@
 
 
 async def get_services_map(
-    director_client: DirectorApi,
+    director_client: DirectorClient,
 ) -> ServiceMetaDataPublishedDict:
 
     # NOTE: using Low-level API to avoid validation
@@ -89,7 +89,7 @@ async def get_services_map(
     key_builder=lambda f, *ag, **kw: f"{f.__name__}/{kw['key']}/{kw['version']}",
 )
 async def get_service(
-    director_client: DirectorApi,
+    director_client: DirectorClient,
     *,
     key: ServiceKey,
     version: ServiceVersion,
@@ -110,7 +110,7 @@ async def get_service(
 
 async def get_batch_services(
     selection: list[tuple[ServiceKey, ServiceVersion]],
-    director_client: DirectorApi,
+    director_client: DirectorClient,
 ) -> list[ServiceMetaDataPublished | BaseException]:
 
     batch: list[ServiceMetaDataPublished | BaseException] = await limited_gather(
diff --git a/services/catalog/src/simcore_service_catalog/service/services.py b/services/catalog/src/simcore_service_catalog/service/services.py
@@ -24,7 +24,7 @@
 )
 from simcore_service_catalog.repository.groups import GroupsRepository
 
-from ..clients.director import DirectorApi
+from ..clients.director import DirectorClient
 from ..models.services_db import (
     ServiceAccessRightsAtDB,
     ServiceMetaDataDBPatch,
@@ -122,7 +122,7 @@ def _to_get_schema(
 
 async def list_latest_services(
     repo: ServicesRepository,
-    director_api: DirectorApi,
+    director_api: DirectorClient,
     *,
     product_name: ProductName,
     user_id: UserID,
@@ -164,6 +164,10 @@ async def list_latest_services(
         if isinstance(sc, ServiceMetaDataPublished)
     }
 
+    # FIXME: services (key, version) matches not found in manifest means that they are in the database but they
+    # do not really exist in the service. We cannot include them but we should definitively warn!!
+    # All services in database should be include in the registry. The background task should be responsible of that
+
     items = [
         _to_latest_get_schema(
             service_db=sc,
@@ -182,7 +186,7 @@ async def list_latest_services(
 
 async def get_service(
     repo: ServicesRepository,
-    director_api: DirectorApi,
+    director_api: DirectorClient,
     product_name: ProductName,
     user_id: UserID,
     service_key: ServiceKey,
@@ -237,7 +241,7 @@ async def get_service(
 
 async def update_service(
     repo: ServicesRepository,
-    director_api: DirectorApi,
+    director_api: DirectorClient,
     *,
     product_name: ProductName,
     user_id: UserID,
@@ -515,7 +519,7 @@ async def list_my_service_release_history(
 
 
 async def get_service_extras(
-    director_api: DirectorApi, service_key: ServiceKey, service_version: VersionStr
+    director_api: DirectorClient, service_key: ServiceKey, service_version: VersionStr
 ) -> ServiceExtras:
     return await director_api.get_service_extras(
         service_key=service_key, service_version=service_version
diff --git a/services/catalog/tests/unit/test_clients_director.py b/services/catalog/tests/unit/test_clients_director.py
@@ -15,8 +15,8 @@
 from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
 from pytest_simcore.helpers.typing_env import EnvVarsDict
 from respx.router import MockRouter
-from simcore_service_catalog.api._dependencies.director import get_director_api
-from simcore_service_catalog.clients.director import DirectorApi
+from simcore_service_catalog.api._dependencies.director import get_director_client
+from simcore_service_catalog.clients.director import DirectorClient
 
 
 @pytest.fixture
@@ -41,10 +41,10 @@ async def test_director_client_high_level_api(
     app: FastAPI,
 ):
     # gets director client as used in handlers
-    director_api = get_director_api(app)
+    director_api = get_director_client(app)
 
     assert app.state.director_api == director_api
-    assert isinstance(director_api, DirectorApi)
+    assert isinstance(director_api, DirectorClient)
 
     # PING
     assert await director_api.is_responsive()
@@ -68,7 +68,7 @@ async def test_director_client_low_level_api(
     expected_director_rest_api_list_services: list[dict[str, Any]],
     app: FastAPI,
 ):
-    director_api = get_director_api(app)
+    director_api = get_director_client(app)
 
     expected_service = expected_director_rest_api_list_services[0]
     key = expected_service["key"]
diff --git a/services/catalog/tests/unit/test_service_manifest.py b/services/catalog/tests/unit/test_service_manifest.py
@@ -13,8 +13,8 @@
 from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
 from pytest_simcore.helpers.typing_env import EnvVarsDict
 from respx.router import MockRouter
-from simcore_service_catalog.api._dependencies.director import get_director_api
-from simcore_service_catalog.clients.director import DirectorApi
+from simcore_service_catalog.api._dependencies.director import get_director_client
+from simcore_service_catalog.clients.director import DirectorClient
 from simcore_service_catalog.service import manifest
 
 
@@ -37,10 +37,10 @@ async def test_services_manifest_api(
     mocked_director_rest_api: MockRouter,
     app: FastAPI,
 ):
-    director_api = get_director_api(app)
+    director_api = get_director_client(app)
 
     assert app.state.director_api == director_api
-    assert isinstance(director_api, DirectorApi)
+    assert isinstance(director_api, DirectorClient)
 
     # LIST
     all_services_map = await manifest.get_services_map(director_api)
diff --git a/services/catalog/tests/unit/with_dbs/test_service_services.py b/services/catalog/tests/unit/with_dbs/test_service_services.py
