From d2a0f16b8bec09d4c5f547480aa691b90435dbd5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 3 Jun 2025 16:11:08 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=8E=A8=20adds=20memory=20cache=20to?= =?UTF-8?q?=20get=5Fservice?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../catalog/_catalog_rest_client_service.py | 29 +++++++++++++++---- .../catalog/_service.py | 24 ++++++++++++--- .../projects/_projects_service.py | 6 +++- .../_pricing_plans_admin_service.py | 6 +++- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py index ac2e37836d59..97dd17fbbf4c 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py @@ -2,10 +2,11 @@ import logging import urllib.parse -from collections.abc import Iterator +from collections.abc import Callable, Iterator from contextlib import contextmanager -from typing import Any +from typing import Any, Final +from aiocache import Cache, cached # type: ignore[import-untyped] from aiohttp import ClientSession, ClientTimeout, web from aiohttp.client_exceptions import ( ClientConnectionError, @@ -15,7 +16,9 @@ from models_library.api_schemas_catalog.service_access_rights import ( ServiceAccessRightsGet, ) +from models_library.products import ProductName from models_library.services_resources import ServiceResourcesDict +from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import TypeAdapter from servicelib.aiohttp import status @@ -29,6 +32,16 @@ _logger = logging.getLogger(__name__) +# Cache settings +_SECOND = 1 # in seconds +_MINUTE = 60 * _SECOND +_CACHE_TTL: Final = 1 * _MINUTE + + +def _hash_service(_f: Callable[..., Any], *_args, **kw): + assert len(_args) == 1, f"Expected only app, got {_args}" # nosec + return f"get_service_{kw['user_id']}_{kw['service_key']}_{kw['service_version']}_{kw['product_name']}" + @contextmanager def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]: @@ -103,12 +116,18 @@ async def get_services_for_user_in_product( return body +@cached( + ttl=_CACHE_TTL, + key_builder=_hash_service, + cache=Cache.MEMORY, +) async def get_service( app: web.Application, + *, user_id: UserID, - service_key: str, - service_version: str, - product_name: str, + service_key: ServiceKey, + service_version: ServiceVersion, + product_name: ProductName, ) -> dict[str, Any]: settings: CatalogSettings = get_plugin_settings(app) url = URL( diff --git a/services/web/server/src/simcore_service_webserver/catalog/_service.py b/services/web/server/src/simcore_service_webserver/catalog/_service.py index daa83c1e5d10..166310ac7fb5 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_service.py @@ -173,7 +173,11 @@ async def list_service_inputs( service_key: ServiceKey, service_version: ServiceVersion, ctx: CatalogRequestContext ) -> list[ServiceInputGet]: service = await _catalog_rest_client_service.get_service( - ctx.app, ctx.user_id, service_key, service_version, ctx.product_name + ctx.app, + user_id=ctx.user_id, + service_key=service_key, + service_version=service_version, + product_name=ctx.product_name, ) return [ await ServiceInputGetFactory.from_catalog_service_api_model( @@ -190,7 +194,11 @@ async def get_service_input( ctx: CatalogRequestContext, ) -> ServiceInputGet: service = await _catalog_rest_client_service.get_service( - ctx.app, ctx.user_id, service_key, service_version, ctx.product_name + ctx.app, + user_id=ctx.user_id, + service_key=service_key, + service_version=service_version, + product_name=ctx.product_name, ) service_input: ServiceInputGet = ( await ServiceInputGetFactory.from_catalog_service_api_model( @@ -249,7 +257,11 @@ async def list_service_outputs( ctx: CatalogRequestContext, ) -> list[ServiceOutputGet]: service = await _catalog_rest_client_service.get_service( - ctx.app, ctx.user_id, service_key, service_version, ctx.product_name + ctx.app, + user_id=ctx.user_id, + service_key=service_key, + service_version=service_version, + product_name=ctx.product_name, ) return [ await ServiceOutputGetFactory.from_catalog_service_api_model( @@ -266,7 +278,11 @@ async def get_service_output( ctx: CatalogRequestContext, ) -> ServiceOutputGet: service = await _catalog_rest_client_service.get_service( - ctx.app, ctx.user_id, service_key, service_version, ctx.product_name + ctx.app, + user_id=ctx.user_id, + service_key=service_key, + service_version=service_version, + product_name=ctx.product_name, ) return cast( # mypy -> aiocache is not typed. ServiceOutputGet, diff --git a/services/web/server/src/simcore_service_webserver/projects/_projects_service.py b/services/web/server/src/simcore_service_webserver/projects/_projects_service.py index 3249048f5f05..4f5dac07a556 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_projects_service.py +++ b/services/web/server/src/simcore_service_webserver/projects/_projects_service.py @@ -1682,7 +1682,11 @@ async def is_service_deprecated( product_name: str, ) -> bool: service = await catalog_service.get_service( - app, user_id, service_key, service_version, product_name + app, + user_id=user_id, + service_key=service_key, + service_version=service_version, + product_name=product_name, ) if deprecation_date := service.get("deprecated"): deprecation_date_bool: bool = datetime.datetime.now( diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_pricing_plans_admin_service.py b/services/web/server/src/simcore_service_webserver/resource_usage/_pricing_plans_admin_service.py index d74bdee870af..60c62f89aafe 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_pricing_plans_admin_service.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_pricing_plans_admin_service.py @@ -164,7 +164,11 @@ async def connect_service_to_pricing_plan( ) -> PricingPlanToServiceGet: # Check whether service key and version exists await catalog_service.get_service( - app, user_id, service_key, service_version, product_name + app, + user_id=user_id, + service_key=service_key, + service_version=service_version, + product_name=product_name, ) rpc_client = get_rabbitmq_rpc_client(app) From 3bea064777b2a63dc6836a1a5c84616758a4edc2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 3 Jun 2025 16:13:39 +0200 Subject: [PATCH 2/4] minor --- .../catalog/_catalog_rest_client_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py index 97dd17fbbf4c..026b085b3d7e 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py @@ -38,7 +38,7 @@ _CACHE_TTL: Final = 1 * _MINUTE -def _hash_service(_f: Callable[..., Any], *_args, **kw): +def _create_service_cache_key_(_f: Callable[..., Any], *_args, **kw): assert len(_args) == 1, f"Expected only app, got {_args}" # nosec return f"get_service_{kw['user_id']}_{kw['service_key']}_{kw['service_version']}_{kw['product_name']}" @@ -118,7 +118,7 @@ async def get_services_for_user_in_product( @cached( ttl=_CACHE_TTL, - key_builder=_hash_service, + key_builder=_create_service_cache_key_, cache=Cache.MEMORY, ) async def get_service( From 3c8d05ec9b7558c310adbb425363a600be5c05f5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 3 Jun 2025 16:50:20 +0200 Subject: [PATCH 3/4] improves tests --- .../catalog/_catalog_rest_client_service.py | 4 +- .../01/test_catalog_handlers__services.py | 151 +++++++++--------- 2 files changed, 80 insertions(+), 75 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py index 026b085b3d7e..cc04740457b1 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py @@ -38,7 +38,7 @@ _CACHE_TTL: Final = 1 * _MINUTE -def _create_service_cache_key_(_f: Callable[..., Any], *_args, **kw): +def _create_service_cache_key(_f: Callable[..., Any], *_args, **kw): assert len(_args) == 1, f"Expected only app, got {_args}" # nosec return f"get_service_{kw['user_id']}_{kw['service_key']}_{kw['service_version']}_{kw['product_name']}" @@ -118,7 +118,7 @@ async def get_services_for_user_in_product( @cached( ttl=_CACHE_TTL, - key_builder=_create_service_cache_key_, + key_builder=_create_service_cache_key, cache=Cache.MEMORY, ) async def get_service( diff --git a/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py b/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py index 7bce1dda8840..e456f9d86f91 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py +++ b/services/web/server/tests/unit/with_dbs/01/test_catalog_handlers__services.py @@ -4,6 +4,7 @@ import re import urllib.parse +from typing import Any import pytest from aiohttp.test_utils import TestClient @@ -68,6 +69,30 @@ def mocked_catalog_rpc_api(mocker: MockerFixture) -> dict[str, MockType]: } +@pytest.fixture +def mocked_catalog_rest_api(aioresponses_mocker: AioResponsesMock) -> dict[str, Any]: + """Fixture that mocks catalog service responses for tests""" + url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") + service_payload = ServiceGetV2.model_json_schema()["examples"][0] + + # Mock multiple responses as needed by tests + for _ in range(6): # Increased to accommodate all tests + aioresponses_mocker.get( + url_pattern, + status=status.HTTP_200_OK, + payload=service_payload, + ) + + service_key = "simcore/services/comp/itis/sleeper" + service_version = "0.1.0" + + return { + "service_key": service_key, + "service_version": service_version, + "service_payload": service_payload, + } + + @pytest.mark.parametrize( "user_role", [UserRole.USER], @@ -101,20 +126,16 @@ async def test_list_services_latest( [UserRole.USER], ) async def test_list_inputs( - client: TestClient, logged_user: UserInfoDict, aioresponses_mocker: AioResponsesMock + client: TestClient, + logged_user: UserInfoDict, + mocked_catalog_rest_api: dict[str, Any], ): + assert client.app + assert client.app.router - url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") - service_payload = ServiceGetV2.model_json_schema()["examples"][0] - aioresponses_mocker.get( - url_pattern, - status=status.HTTP_200_OK, - payload=service_payload, - ) + service_key = mocked_catalog_rest_api["service_key"] + service_version = mocked_catalog_rest_api["service_version"] - service_key = "simcore/services/comp/itis/sleeper" - service_version = "0.1.0" - assert client.app and client.app.router url = client.app.router["list_service_inputs"].url_for( service_key=urllib.parse.quote(service_key, safe=""), service_version=service_version, @@ -130,20 +151,16 @@ async def test_list_inputs( [UserRole.USER], ) async def test_list_outputs( - client: TestClient, logged_user: UserInfoDict, aioresponses_mocker: AioResponsesMock + client: TestClient, + logged_user: UserInfoDict, + mocked_catalog_rest_api: dict[str, Any], ): + assert client.app + assert client.app.router - url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") - service_payload = ServiceGetV2.model_json_schema()["examples"][0] - aioresponses_mocker.get( - url_pattern, - status=status.HTTP_200_OK, - payload=service_payload, - ) + service_key = mocked_catalog_rest_api["service_key"] + service_version = mocked_catalog_rest_api["service_version"] - service_key = "simcore/services/comp/itis/sleeper" - service_version = "0.1.0" - assert client.app and client.app.router url = client.app.router["list_service_outputs"].url_for( service_key=urllib.parse.quote(service_key, safe=""), service_version=service_version, @@ -159,20 +176,17 @@ async def test_list_outputs( [UserRole.USER], ) async def test_get_outputs( - client: TestClient, logged_user: UserInfoDict, aioresponses_mocker: AioResponsesMock + client: TestClient, + logged_user: UserInfoDict, + mocked_catalog_rest_api: dict[str, Any], ): + assert client.app + assert client.app.router - url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") - service_payload = ServiceGetV2.model_json_schema()["examples"][0] - aioresponses_mocker.get( - url_pattern, - status=status.HTTP_200_OK, - payload=service_payload, - ) + service_key = mocked_catalog_rest_api["service_key"] + service_version = mocked_catalog_rest_api["service_version"] + service_payload = mocked_catalog_rest_api["service_payload"] - service_key = "simcore/services/comp/itis/sleeper" - service_version = "0.1.0" - assert client.app and client.app.router url = client.app.router["get_service_output"].url_for( service_key=urllib.parse.quote(service_key, safe=""), service_version=service_version, @@ -189,19 +203,17 @@ async def test_get_outputs( [UserRole.USER], ) async def test_get_inputs( - client: TestClient, logged_user: UserInfoDict, aioresponses_mocker: AioResponsesMock + client: TestClient, + logged_user: UserInfoDict, + mocked_catalog_rest_api: dict[str, Any], ): - url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") - service_payload = ServiceGetV2.model_json_schema()["examples"][0] - aioresponses_mocker.get( - url_pattern, - status=status.HTTP_200_OK, - payload=service_payload, - ) + assert client.app + assert client.app.router + + service_key = mocked_catalog_rest_api["service_key"] + service_version = mocked_catalog_rest_api["service_version"] + service_payload = mocked_catalog_rest_api["service_payload"] - service_key = "simcore/services/comp/itis/sleeper" - service_version = "0.1.0" - assert client.app and client.app.router url = client.app.router["get_service_input"].url_for( service_key=urllib.parse.quote(service_key, safe=""), service_version=service_version, @@ -217,20 +229,17 @@ async def test_get_inputs( [UserRole.USER], ) async def test_get_compatible_inputs_given_source_outputs( - client: TestClient, logged_user: UserInfoDict, aioresponses_mocker: AioResponsesMock + client: TestClient, + logged_user: UserInfoDict, + mocked_catalog_rest_api: dict[str, Any], ): - url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") - service_payload = ServiceGetV2.model_json_schema()["examples"][0] - for _ in range(2): - aioresponses_mocker.get( - url_pattern, - status=status.HTTP_200_OK, - payload=service_payload, - ) + assert client.app + assert client.app.router - service_key = "simcore/services/comp/itis/sleeper" - service_version = "0.1.0" - assert client.app and client.app.router + service_key = mocked_catalog_rest_api["service_key"] + service_version = mocked_catalog_rest_api["service_version"] + + # Get compatible inputs given source outputs url = ( client.app.router["get_compatible_inputs_given_source_output"] .url_for( @@ -239,8 +248,8 @@ async def test_get_compatible_inputs_given_source_outputs( ) .with_query( { - "fromService": "simcore/services/comp/itis/sleeper", - "fromVersion": "0.1.0", + "fromService": service_key, + "fromVersion": service_version, "fromOutput": "output_1", } ) @@ -253,21 +262,17 @@ async def test_get_compatible_inputs_given_source_outputs( "user_role", [UserRole.USER], ) -async def test_get_compatible_outputs_given_target_inptuts( - client: TestClient, logged_user: UserInfoDict, aioresponses_mocker: AioResponsesMock +async def test_get_compatible_outputs_given_target_inputs( + client: TestClient, + logged_user: UserInfoDict, + mocked_catalog_rest_api: dict[str, Any], ): - url_pattern = re.compile(r"http://catalog:8000/v0/services/.*") - service_payload = ServiceGetV2.model_json_schema()["examples"][0] - for _ in range(2): - aioresponses_mocker.get( - url_pattern, - status=status.HTTP_200_OK, - payload=service_payload, - ) + assert client.app + assert client.app.router + + service_key = mocked_catalog_rest_api["service_key"] + service_version = mocked_catalog_rest_api["service_version"] - service_key = "simcore/services/comp/itis/sleeper" - service_version = "0.1.0" - assert client.app and client.app.router url = ( client.app.router["get_compatible_outputs_given_target_input"] .url_for( @@ -276,8 +281,8 @@ async def test_get_compatible_outputs_given_target_inptuts( ) .with_query( { - "toService": "simcore/services/comp/itis/sleeper", - "toVersion": "0.1.0", + "toService": service_key, + "toVersion": service_version, "toInput": "input_1", } ) From 8a9f0a0ee8400256e5990e6e36a18ef6c5c32ef5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 3 Jun 2025 16:51:59 +0200 Subject: [PATCH 4/4] @sanderegg review: doc --- .../catalog/_catalog_rest_client_service.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py index cc04740457b1..99c630ed8fa7 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py @@ -120,6 +120,7 @@ async def get_services_for_user_in_product( ttl=_CACHE_TTL, key_builder=_create_service_cache_key, cache=Cache.MEMORY, + # SEE https://github.com/ITISFoundation/osparc-simcore/pull/7802 ) async def get_service( app: web.Application,