From 5dfa5504af2c1eca3a096d0a89bf0e198205315b Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Mon, 14 Apr 2025 16:24:17 +0200 Subject: [PATCH 01/28] _service.py -> service_jobs.py --- .../{_service.py => _service_jobs.py} | 2 +- .../src/simcore_service_api_server/api/routes/programs.py | 4 ++-- .../src/simcore_service_api_server/api/routes/solvers_jobs.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) rename services/api-server/src/simcore_service_api_server/{_service.py => _service_jobs.py} (98%) diff --git a/services/api-server/src/simcore_service_api_server/_service.py b/services/api-server/src/simcore_service_api_server/_service_jobs.py similarity index 98% rename from services/api-server/src/simcore_service_api_server/_service.py rename to services/api-server/src/simcore_service_api_server/_service_jobs.py index bc826b1a3105..bf84506bebc9 100644 --- a/services/api-server/src/simcore_service_api_server/_service.py +++ b/services/api-server/src/simcore_service_api_server/_service_jobs.py @@ -17,7 +17,7 @@ _logger = logging.getLogger(__name__) -async def create_solver_or_program_job( +async def create_job( *, webserver_api: AuthSession, solver_or_program: Solver | Program, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index 5f69580e06b8..e9190e6c3d3f 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -17,7 +17,7 @@ complete_file_upload, get_upload_links_from_s3, ) -from simcore_service_api_server._service import create_solver_or_program_job +from simcore_service_api_server._service_jobs import create_job from simcore_service_api_server.api.dependencies.webserver_http import ( get_webserver_session, ) @@ -130,7 +130,7 @@ async def create_program_job( product_name=product_name, ) - job, project = await create_solver_or_program_job( + job, project = await create_job( webserver_api=webserver_api, solver_or_program=program, inputs=inputs, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py index 3fc1b1109ade..d1569406abc5 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py @@ -12,7 +12,7 @@ from models_library.projects_nodes_io import NodeID from pydantic.types import PositiveInt -from ..._service import create_solver_or_program_job +from ..._service_jobs import create_job from ...exceptions.backend_errors import ProjectAlreadyStartedError from ...exceptions.service_errors_utils import DEFAULT_BACKEND_SERVICE_STATUS_CODES from ...models.basic_types import VersionStr @@ -116,7 +116,7 @@ async def create_solver_job( version=version, product_name=product_name, ) - job, project = await create_solver_or_program_job( + job, project = await create_job( webserver_api=webserver_api, solver_or_program=solver, inputs=inputs, From e894814262d17a801ec08af1cd64f3e1f91b4214 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Mon, 14 Apr 2025 17:28:39 +0200 Subject: [PATCH 02/28] start program service --- .../_service_programs.py | 34 +++++++++++++++++++ .../models/schemas/programs.py | 14 ++++++++ 2 files changed, 48 insertions(+) create mode 100644 services/api-server/src/simcore_service_api_server/_service_programs.py diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py new file mode 100644 index 000000000000..f421409af3a5 --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -0,0 +1,34 @@ +from dataclasses import dataclass + +from models_library.basic_types import VersionStr +from models_library.services_enums import ServiceType +from servicelib.fastapi.app_state import SingletonInAppStateMixin +from simcore_service_api_server.models.schemas.programs import Program, ProgramKeyId + +from .services_rpc.catalog import CatalogService + + +@dataclass +class ProgramService(SingletonInAppStateMixin): + app_state_name = "ProgramService" + + async def get_program( + self, + *, + catalog_service: CatalogService, + user_id: int, + name: ProgramKeyId, + version: VersionStr, + product_name: str, + ) -> Program: + service = await catalog_service.get( + user_id=user_id, + service_key=name, + service_version=version, + product_name=product_name, + ) + assert ( # nosec + service.service_type == ServiceType.DYNAMIC + ), "Expected by ProgramName regex" + + return Program.create_from_service(service) diff --git a/services/api-server/src/simcore_service_api_server/models/schemas/programs.py b/services/api-server/src/simcore_service_api_server/models/schemas/programs.py index 25d1d1f5cf26..719af8a839e9 100644 --- a/services/api-server/src/simcore_service_api_server/models/schemas/programs.py +++ b/services/api-server/src/simcore_service_api_server/models/schemas/programs.py @@ -1,5 +1,6 @@ from typing import Annotated +from models_library.api_schemas_catalog.services import ServiceGetV2 from models_library.services import ServiceMetaDataPublished from models_library.services_regex import DYNAMIC_SERVICE_KEY_RE from pydantic import ConfigDict, StringConstraints @@ -56,6 +57,19 @@ def create_from_image(cls, image_meta: ServiceMetaDataPublished) -> "Program": **data, ) + @classmethod + def create_from_service(cls, service: ServiceGetV2) -> "Program": + data = service.model_dump( + include={"name", "key", "version", "description", "contact"}, + ) + return cls( + id=data.pop("key"), + version=data.pop("version"), + title=data.pop("name"), + url=None, + **data, + ) + @classmethod def compose_resource_name(cls, key: ProgramKeyId, version: VersionStr) -> str: return compose_resource_name("programs", key, "releases", version) From 9923b390aab561537500ea8a01c63af971786910 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 09:10:59 +0200 Subject: [PATCH 03/28] create program service --- .../_service_programs.py | 18 ++++++++++++------ .../api/dependencies/program_service.py | 11 +++++++++++ .../api/routes/programs.py | 14 ++++++++------ .../core/application.py | 3 +++ .../services_http/rabbitmq.py | 9 +++++++-- .../services_rpc/catalog.py | 6 ++++++ 6 files changed, 47 insertions(+), 14 deletions(-) create mode 100644 services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py index f421409af3a5..a239613bcb22 100644 --- a/services/api-server/src/simcore_service_api_server/_service_programs.py +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -1,34 +1,40 @@ from dataclasses import dataclass +from fastapi import FastAPI from models_library.basic_types import VersionStr from models_library.services_enums import ServiceType from servicelib.fastapi.app_state import SingletonInAppStateMixin -from simcore_service_api_server.models.schemas.programs import Program, ProgramKeyId +from .models.schemas.programs import Program, ProgramKeyId from .services_rpc.catalog import CatalogService @dataclass class ProgramService(SingletonInAppStateMixin): app_state_name = "ProgramService" + _catalog_service: CatalogService async def get_program( self, *, - catalog_service: CatalogService, user_id: int, name: ProgramKeyId, version: VersionStr, product_name: str, ) -> Program: - service = await catalog_service.get( + service = await self._catalog_service.get( user_id=user_id, service_key=name, service_version=version, product_name=product_name, ) - assert ( # nosec - service.service_type == ServiceType.DYNAMIC - ), "Expected by ProgramName regex" + assert service.service_type == ServiceType.DYNAMIC # nosec return Program.create_from_service(service) + + +def setup(app: FastAPI): + _catalog_service = CatalogService.get_from_app_state(app) + assert isinstance(_catalog_service, CatalogService) # nosec + program_service = ProgramService(_catalog_service=_catalog_service) + program_service.set_to_app_state(app=app) diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py new file mode 100644 index 000000000000..f382faed690b --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py @@ -0,0 +1,11 @@ +from typing import Annotated + +from fastapi import Depends, FastAPI +from servicelib.fastapi.dependencies import get_app +from simcore_service_api_server._service_programs import ProgramService + + +async def get_program_service( + app: Annotated[FastAPI, Depends(get_app)], +) -> ProgramService: + return ProgramService.get_from_app_state(app=app) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index e9190e6c3d3f..f475fa1a07b7 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -17,16 +17,18 @@ complete_file_upload, get_upload_links_from_s3, ) -from simcore_service_api_server._service_jobs import create_job -from simcore_service_api_server.api.dependencies.webserver_http import ( + +from ..._service_jobs import create_job +from ..._service_programs import ProgramService +from ...api.dependencies.program_service import get_program_service +from ...api.dependencies.webserver_http import ( get_webserver_session, ) -from simcore_service_api_server.services_http.webserver import AuthSession - from ...models.basic_types import VersionStr from ...models.schemas.jobs import Job, JobInputs from ...models.schemas.programs import Program, ProgramKeyId from ...services_http.catalog import CatalogApi +from ...services_http.webserver import AuthSession from ..dependencies.authentication import get_current_user_id, get_product_name from ..dependencies.services import get_api_client @@ -70,13 +72,13 @@ async def get_program_release( program_key: ProgramKeyId, version: VersionStr, user_id: Annotated[int, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + program_service: Annotated[ProgramService, Depends(get_program_service)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ) -> Program: """Gets a specific release of a solver""" try: - program = await catalog_client.get_program( + program = await program_service.get_program( user_id=user_id, name=program_key, version=version, diff --git a/services/api-server/src/simcore_service_api_server/core/application.py b/services/api-server/src/simcore_service_api_server/core/application.py index cde8280313ac..e64bcfdd91bd 100644 --- a/services/api-server/src/simcore_service_api_server/core/application.py +++ b/services/api-server/src/simcore_service_api_server/core/application.py @@ -10,6 +10,7 @@ from .. import exceptions from .._meta import API_VERSION, API_VTAG, APP_NAME +from .._service_programs import setup as setup_program_service from ..api.root import create_router from ..api.routes.health import router as health_router from ..services_http import catalog, director_v2, storage, webserver @@ -83,6 +84,8 @@ def init_app(settings: ApplicationSettings | None = None) -> FastAPI: setup_rabbitmq(app) + setup_program_service(app) + if settings.API_SERVER_TRACING: initialize_tracing(app, settings.API_SERVER_TRACING, APP_NAME) diff --git a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py index d2daaf824342..52e5d0f8d8e0 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py +++ b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py @@ -5,10 +5,11 @@ from servicelib.rabbitmq._client_rpc import RabbitMQRPCClient from settings_library.rabbit import RabbitSettings +from .._service_programs import setup as setup_program_service from ..api.dependencies.rabbitmq import get_rabbitmq_rpc_client from ..core.health_checker import ApiServerHealthChecker from ..services_http.log_streaming import LogDistributor -from ..services_rpc import resource_usage_tracker, wb_api_server +from ..services_rpc import catalog, resource_usage_tracker, wb_api_server _logger = logging.getLogger(__name__) @@ -38,8 +39,12 @@ async def _on_startup() -> None: await app.state.health_checker.setup( app.state.settings.API_SERVER_HEALTH_CHECK_TASK_PERIOD_SECONDS ) - wb_api_server.setup(app, get_rabbitmq_rpc_client(app)) + # setup rpc clients + catalog.setup(app, get_rabbitmq_rpc_client(app)) resource_usage_tracker.setup(app, get_rabbitmq_rpc_client(app)) + wb_api_server.setup(app, get_rabbitmq_rpc_client(app)) + + setup_program_service(app) async def _on_shutdown() -> None: if app.state.health_checker: diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index 0a9fde924dee..d2da9bbd8bd3 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -1,5 +1,6 @@ from dataclasses import dataclass +from fastapi import FastAPI from models_library.api_schemas_catalog.services import LatestServiceGet, ServiceGetV2 from models_library.products import ProductName from models_library.rest_pagination import ( @@ -87,3 +88,8 @@ async def get( service_key=service_key, service_version=service_version, ) + + +def setup(app: FastAPI, rabbitmq_rmp_client: RabbitMQRPCClient): + catalog_service = CatalogService(_client=rabbitmq_rmp_client) + catalog_service.set_to_app_state(app=app) From be7bff6fe01df6a26ad97f2bb0e192647af43c7d Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 09:54:07 +0200 Subject: [PATCH 04/28] add unit test for getting program --- .../helpers/catalog_rpc_server.py | 13 +++++ .../core/application.py | 3 -- .../services_http/rabbitmq.py | 1 + services/api-server/tests/unit/conftest.py | 37 ++++++++++++++ .../tests/unit/test_api_programs.py | 49 +++++++++++++++++++ .../tests/unit/test_services_catalog.py | 36 +------------- 6 files changed, 101 insertions(+), 38 deletions(-) create mode 100644 services/api-server/tests/unit/test_api_programs.py diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py index aa6db574410a..fe112184b0ee 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/catalog_rpc_server.py @@ -12,7 +12,12 @@ from models_library.products import ProductName from models_library.rest_pagination import PageOffsetInt from models_library.rpc_pagination import PageLimitInt, PageRpc +from models_library.services_enums import ServiceType from models_library.services_history import ServiceRelease +from models_library.services_regex import ( + COMPUTATIONAL_SERVICE_KEY_RE, + DYNAMIC_SERVICE_KEY_RE, +) from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID from pydantic import NonNegativeInt, TypeAdapter @@ -65,6 +70,14 @@ async def get_service( got.version = service_version got.key = service_key + if DYNAMIC_SERVICE_KEY_RE.match(got.key): + got.service_type = ServiceType.DYNAMIC + elif COMPUTATIONAL_SERVICE_KEY_RE.match(got.key): + got.service_type = ServiceType.COMPUTATIONAL + else: + msg = "Service type not recognized. Please extend the mock yourself" + raise RuntimeError(msg) + return got async def update_service( diff --git a/services/api-server/src/simcore_service_api_server/core/application.py b/services/api-server/src/simcore_service_api_server/core/application.py index e64bcfdd91bd..cde8280313ac 100644 --- a/services/api-server/src/simcore_service_api_server/core/application.py +++ b/services/api-server/src/simcore_service_api_server/core/application.py @@ -10,7 +10,6 @@ from .. import exceptions from .._meta import API_VERSION, API_VTAG, APP_NAME -from .._service_programs import setup as setup_program_service from ..api.root import create_router from ..api.routes.health import router as health_router from ..services_http import catalog, director_v2, storage, webserver @@ -84,8 +83,6 @@ def init_app(settings: ApplicationSettings | None = None) -> FastAPI: setup_rabbitmq(app) - setup_program_service(app) - if settings.API_SERVER_TRACING: initialize_tracing(app, settings.API_SERVER_TRACING, APP_NAME) diff --git a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py index 52e5d0f8d8e0..c62dcb90c211 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py +++ b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py @@ -44,6 +44,7 @@ async def _on_startup() -> None: resource_usage_tracker.setup(app, get_rabbitmq_rpc_client(app)) wb_api_server.setup(app, get_rabbitmq_rpc_client(app)) + # setup dependent services layers setup_program_service(app) async def _on_shutdown() -> None: diff --git a/services/api-server/tests/unit/conftest.py b/services/api-server/tests/unit/conftest.py index 8fb9fb2a4452..7396f582dcc2 100644 --- a/services/api-server/tests/unit/conftest.py +++ b/services/api-server/tests/unit/conftest.py @@ -38,6 +38,7 @@ from packaging.version import Version from pydantic import EmailStr, HttpUrl, TypeAdapter from pytest_mock import MockerFixture, MockType +from pytest_simcore.helpers.catalog_rpc_server import CatalogRpcSideEffects from pytest_simcore.helpers.host import get_localhost_ip from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict from pytest_simcore.helpers.webserver_rpc_server import WebserverRpcSideEffects @@ -45,12 +46,48 @@ from requests.auth import HTTPBasicAuth from respx import MockRouter from servicelib.rabbitmq._client_rpc import RabbitMQRPCClient +from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc from simcore_service_api_server.core.application import init_app from simcore_service_api_server.core.settings import ApplicationSettings from simcore_service_api_server.db.repositories.api_keys import UserAndProductTuple from simcore_service_api_server.services_http.solver_job_outputs import ResultsTypes +@pytest.fixture +def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType]: + """ + Mocks the RPC catalog service API for testing purposes. + """ + side_effects = CatalogRpcSideEffects() + + return { + "list_services_paginated": mocker.patch.object( + catalog_rpc, + "list_services_paginated", + autospec=True, + side_effect=side_effects.list_services_paginated, + ), + "get_service": mocker.patch.object( + catalog_rpc, + "get_service", + autospec=True, + side_effect=side_effects.get_service, + ), + "update_service": mocker.patch.object( + catalog_rpc, + "update_service", + autospec=True, + side_effect=side_effects.update_service, + ), + "list_my_service_history_paginated": mocker.patch.object( + catalog_rpc, + "list_my_service_history_paginated", + autospec=True, + side_effect=side_effects.list_my_service_history_paginated, + ), + } + + @pytest.fixture def app_environment( monkeypatch: pytest.MonkeyPatch, diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py new file mode 100644 index 000000000000..179268a6eec4 --- /dev/null +++ b/services/api-server/tests/unit/test_api_programs.py @@ -0,0 +1,49 @@ +import httpx +import pytest +from fastapi import FastAPI, status +from httpx import AsyncClient +from models_library.users import UserID +from pytest_mock import MockerFixture, MockType +from simcore_service_api_server._meta import API_VTAG +from simcore_service_api_server._service_programs import ProgramService +from simcore_service_api_server.api.dependencies.program_service import ( + get_program_service, +) +from simcore_service_api_server.models.schemas.programs import Program +from simcore_service_api_server.services_rpc.catalog import CatalogService + + +@pytest.fixture +def mock_program_service(mocker: MockerFixture, app: FastAPI): + + def _get_program_service(): + catalog_service = CatalogService(_client=mocker.MagicMock()) + return ProgramService(_catalog_service=catalog_service) + + app.dependency_overrides[get_program_service] = _get_program_service + yield + app.dependency_overrides.pop(get_program_service) + + +@pytest.mark.asyncio +async def test_get_program_release( + user_id: UserID, + mocker: MockerFixture, + client: AsyncClient, + mocked_rpc_catalog_service_api: dict[str, MockType], + mock_program_service: None, + auth: httpx.BasicAuth, +): + # Arrange + program_key = "simcore/services/dynamic/my_program" + version = "1.0.0" + + response = await client.get( + f"/{API_VTAG}/programs/{program_key}/releases/{version}", auth=auth + ) + + # Assert + assert response.status_code == status.HTTP_200_OK + program = Program.model_validate(response.json()) + assert program.id == program_key + assert program.version == version diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index 99c5dc58b4cd..ae2cb4145e65 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -12,9 +12,8 @@ from models_library.users import UserID from pydantic import HttpUrl from pytest_mock import MockerFixture, MockType -from pytest_simcore.helpers.catalog_rpc_server import CatalogRpcSideEffects from simcore_service_api_server.models.schemas.solvers import Solver -from simcore_service_api_server.services_rpc.catalog import CatalogService, catalog_rpc +from simcore_service_api_server.services_rpc.catalog import CatalogService @pytest.fixture @@ -22,39 +21,6 @@ def product_name() -> ProductName: return "osparc" -@pytest.fixture -def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType]: - - side_effects = CatalogRpcSideEffects() - - return { - "list_services_paginated": mocker.patch.object( - catalog_rpc, - "list_services_paginated", - autospec=True, - side_effect=side_effects.list_services_paginated, - ), - "get_service": mocker.patch.object( - catalog_rpc, - "get_service", - autospec=True, - side_effect=side_effects.get_service, - ), - "update_service": mocker.patch.object( - catalog_rpc, - "update_service", - autospec=True, - side_effect=side_effects.update_service, - ), - "list_my_service_history_paginated": mocker.patch.object( - catalog_rpc, - "list_my_service_history_paginated", - autospec=True, - side_effect=side_effects.list_my_service_history_paginated, - ), - } - - def to_solver( service: LatestServiceGet | ServiceGetV2, href_self: HttpUrl | None = None ) -> Solver: From ca5e1590de8dc6cc61d808b0faf8937a0d1efcb2 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 09:56:33 +0200 Subject: [PATCH 05/28] cleanup function arguments --- services/api-server/tests/unit/test_api_programs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index 179268a6eec4..9d4523107d00 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -27,12 +27,12 @@ def _get_program_service(): @pytest.mark.asyncio async def test_get_program_release( - user_id: UserID, - mocker: MockerFixture, + auth: httpx.BasicAuth, client: AsyncClient, mocked_rpc_catalog_service_api: dict[str, MockType], + mocker: MockerFixture, mock_program_service: None, - auth: httpx.BasicAuth, + user_id: UserID, ): # Arrange program_key = "simcore/services/dynamic/my_program" From 9e7bd41eb7ee1487fe8d08e9f004bb2ef9a768b9 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 11:43:28 +0200 Subject: [PATCH 06/28] introduce job_service and 'constuct' service layers in dependency injectsions --- .../_service_job.py | 75 +++++++++++++++++++ .../_service_jobs.py | 58 -------------- .../_service_programs.py | 14 ++-- .../api/dependencies/catalog_service.py | 13 ++++ .../api/dependencies/job_service.py | 17 +++++ .../api/dependencies/program_service.py | 10 ++- .../api/routes/programs.py | 19 ++--- .../api/routes/solvers_jobs.py | 8 +- .../core/application.py | 4 + .../services_http/rabbitmq.py | 4 - 10 files changed, 137 insertions(+), 85 deletions(-) create mode 100644 services/api-server/src/simcore_service_api_server/_service_job.py delete mode 100644 services/api-server/src/simcore_service_api_server/_service_jobs.py create mode 100644 services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py create mode 100644 services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py diff --git a/services/api-server/src/simcore_service_api_server/_service_job.py b/services/api-server/src/simcore_service_api_server/_service_job.py new file mode 100644 index 000000000000..fe378a9d8dcd --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/_service_job.py @@ -0,0 +1,75 @@ +import logging +from collections.abc import Callable +from dataclasses import dataclass + +from fastapi import FastAPI +from models_library.api_schemas_webserver.projects import ProjectCreateNew, ProjectGet +from models_library.projects import ProjectID +from models_library.projects_nodes_io import NodeID +from pydantic import HttpUrl +from servicelib.fastapi.app_state import SingletonInAppStateMixin +from simcore_service_api_server.models.schemas.jobs import Job, JobInputs +from simcore_service_api_server.models.schemas.programs import Program +from simcore_service_api_server.models.schemas.solvers import Solver +from simcore_service_api_server.services_http.solver_job_models_converters import ( + create_job_from_project, + create_new_project_for_job, +) +from simcore_service_api_server.services_http.webserver import AuthSession + +_logger = logging.getLogger(__name__) + + +@dataclass() +class JobService(SingletonInAppStateMixin): + app_state_name = "JobService" + _webserver_api: AuthSession | None = None + + @property + def webserver_api(self) -> AuthSession: + assert self._webserver_api, "JobService not initialized" # nosec + return self._webserver_api + + async def create_job( + self, + *, + solver_or_program: Solver | Program, + inputs: JobInputs, + parent_project_uuid: ProjectID | None, + parent_node_id: NodeID | None, + url_for: Callable[..., HttpUrl], + hidden: bool + ) -> tuple[Job, ProjectGet]: + # creates NEW job as prototype + pre_job = Job.create_job_from_solver_or_program( + solver_or_program_name=solver_or_program.name, inputs=inputs + ) + _logger.debug("Creating Job '%s'", pre_job.name) + + project_in: ProjectCreateNew = create_new_project_for_job( + solver_or_program, pre_job, inputs + ) + new_project: ProjectGet = await self.webserver_api.create_project( + project_in, + is_hidden=hidden, + parent_project_uuid=parent_project_uuid, + parent_node_id=parent_node_id, + ) + assert new_project # nosec + assert new_project.uuid == pre_job.id # nosec + + # for consistency, it rebuild job + job = create_job_from_project( + solver_or_program=solver_or_program, project=new_project, url_for=url_for + ) + assert job.id == pre_job.id # nosec + assert job.name == pre_job.name # nosec + assert job.name == Job.compose_resource_name( + parent_name=solver_or_program.resource_name, + job_id=job.id, + ) + return job, new_project + + +def setup(app: FastAPI): + JobService().set_to_app_state(app=app) diff --git a/services/api-server/src/simcore_service_api_server/_service_jobs.py b/services/api-server/src/simcore_service_api_server/_service_jobs.py deleted file mode 100644 index bf84506bebc9..000000000000 --- a/services/api-server/src/simcore_service_api_server/_service_jobs.py +++ /dev/null @@ -1,58 +0,0 @@ -import logging -from collections.abc import Callable - -from models_library.api_schemas_webserver.projects import ProjectCreateNew, ProjectGet -from models_library.projects import ProjectID -from models_library.projects_nodes_io import NodeID -from pydantic import HttpUrl -from simcore_service_api_server.models.schemas.jobs import Job, JobInputs -from simcore_service_api_server.models.schemas.programs import Program -from simcore_service_api_server.models.schemas.solvers import Solver -from simcore_service_api_server.services_http.solver_job_models_converters import ( - create_job_from_project, - create_new_project_for_job, -) -from simcore_service_api_server.services_http.webserver import AuthSession - -_logger = logging.getLogger(__name__) - - -async def create_job( - *, - webserver_api: AuthSession, - solver_or_program: Solver | Program, - inputs: JobInputs, - parent_project_uuid: ProjectID | None, - parent_node_id: NodeID | None, - url_for: Callable[..., HttpUrl], - hidden: bool -) -> tuple[Job, ProjectGet]: - # creates NEW job as prototype - pre_job = Job.create_job_from_solver_or_program( - solver_or_program_name=solver_or_program.name, inputs=inputs - ) - _logger.debug("Creating Job '%s'", pre_job.name) - - project_in: ProjectCreateNew = create_new_project_for_job( - solver_or_program, pre_job, inputs - ) - new_project: ProjectGet = await webserver_api.create_project( - project_in, - is_hidden=hidden, - parent_project_uuid=parent_project_uuid, - parent_node_id=parent_node_id, - ) - assert new_project # nosec - assert new_project.uuid == pre_job.id # nosec - - # for consistency, it rebuild job - job = create_job_from_project( - solver_or_program=solver_or_program, project=new_project, url_for=url_for - ) - assert job.id == pre_job.id # nosec - assert job.name == pre_job.name # nosec - assert job.name == Job.compose_resource_name( - parent_name=solver_or_program.resource_name, - job_id=job.id, - ) - return job, new_project diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py index a239613bcb22..8dd89133fc8b 100644 --- a/services/api-server/src/simcore_service_api_server/_service_programs.py +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -12,7 +12,12 @@ @dataclass class ProgramService(SingletonInAppStateMixin): app_state_name = "ProgramService" - _catalog_service: CatalogService + _catalog_service: CatalogService | None = None + + @property + def catalog_service(self) -> CatalogService: + assert self._catalog_service, "ProgramService not initialized" # nosec + return self._catalog_service async def get_program( self, @@ -22,7 +27,7 @@ async def get_program( version: VersionStr, product_name: str, ) -> Program: - service = await self._catalog_service.get( + service = await self.catalog_service.get( user_id=user_id, service_key=name, service_version=version, @@ -34,7 +39,4 @@ async def get_program( def setup(app: FastAPI): - _catalog_service = CatalogService.get_from_app_state(app) - assert isinstance(_catalog_service, CatalogService) # nosec - program_service = ProgramService(_catalog_service=_catalog_service) - program_service.set_to_app_state(app=app) + ProgramService().set_to_app_state(app=app) diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py new file mode 100644 index 000000000000..2f07065b1ce3 --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py @@ -0,0 +1,13 @@ +from typing import Annotated + +from fastapi import Depends, FastAPI +from servicelib.fastapi.dependencies import get_app +from simcore_service_api_server.services_rpc.catalog import CatalogService + + +async def get_catalog_service( + app: Annotated[FastAPI, Depends(get_app)], +) -> CatalogService: + catalog_service = CatalogService.get_from_app_state(app=app) + assert isinstance(catalog_service, CatalogService) + return catalog_service diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py new file mode 100644 index 000000000000..37fb41be6185 --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py @@ -0,0 +1,17 @@ +from typing import Annotated + +from fastapi import Depends, FastAPI +from servicelib.fastapi.dependencies import get_app +from simcore_service_api_server._service_job import JobService +from simcore_service_api_server.services_http.webserver import AuthSession + +from .webserver_http import get_webserver_session + + +async def get_job_service( + app: Annotated[FastAPI, Depends(get_app)], + webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], +) -> JobService: + job_service = JobService.get_from_app_state(app=app) + job_service._webserver_api = webserver_api + return job_service diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py index f382faed690b..78192dd20575 100644 --- a/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py +++ b/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py @@ -2,10 +2,16 @@ from fastapi import Depends, FastAPI from servicelib.fastapi.dependencies import get_app -from simcore_service_api_server._service_programs import ProgramService + +from ..._service_programs import ProgramService +from ...services_rpc.catalog import CatalogService +from .catalog_service import get_catalog_service async def get_program_service( app: Annotated[FastAPI, Depends(get_app)], + catalog_service: Annotated[CatalogService, Depends(get_catalog_service)], ) -> ProgramService: - return ProgramService.get_from_app_state(app=app) + _program_service = ProgramService.get_from_app_state(app=app) + _program_service._catalog_service = catalog_service + return _program_service diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index f475fa1a07b7..ad74236e4364 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -18,18 +18,15 @@ get_upload_links_from_s3, ) -from ..._service_jobs import create_job +from ..._service_job import JobService from ..._service_programs import ProgramService from ...api.dependencies.program_service import get_program_service -from ...api.dependencies.webserver_http import ( - get_webserver_session, -) from ...models.basic_types import VersionStr from ...models.schemas.jobs import Job, JobInputs from ...models.schemas.programs import Program, ProgramKeyId from ...services_http.catalog import CatalogApi -from ...services_http.webserver import AuthSession from ..dependencies.authentication import get_current_user_id, get_product_name +from ..dependencies.job_service import get_job_service from ..dependencies.services import get_api_client _logger = logging.getLogger(__name__) @@ -45,7 +42,8 @@ async def list_programs( ): """Lists all available solvers (latest version) - SEE get_solvers_page for paginated version of this function + DEPRECATION: This implementation and returned values are deprecated + """ services = await catalog_client.list_services( user_id=user_id, @@ -111,8 +109,8 @@ async def create_program_job( program_key: ProgramKeyId, version: VersionStr, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], - webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], + program_service: Annotated[ProgramService, Depends(get_program_service)], + job_service: Annotated[JobService, Depends(get_job_service)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], x_simcore_parent_project_uuid: Annotated[ProjectID | None, Header()] = None, @@ -125,15 +123,14 @@ async def create_program_job( # ensures user has access to solver inputs = JobInputs(values={}) - program = await catalog_client.get_program( + program = await program_service.get_program( user_id=user_id, name=program_key, version=version, product_name=product_name, ) - job, project = await create_job( - webserver_api=webserver_api, + job, project = await job_service.create_job( solver_or_program=program, inputs=inputs, parent_project_uuid=x_simcore_parent_project_uuid, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py index d1569406abc5..9d101c69f61c 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py @@ -12,7 +12,7 @@ from models_library.projects_nodes_io import NodeID from pydantic.types import PositiveInt -from ..._service_jobs import create_job +from ..._service_job import JobService from ...exceptions.backend_errors import ProjectAlreadyStartedError from ...exceptions.service_errors_utils import DEFAULT_BACKEND_SERVICE_STATUS_CODES from ...models.basic_types import VersionStr @@ -35,6 +35,7 @@ from ...services_rpc.wb_api_server import WbApiRpcClient from ..dependencies.application import get_reverse_url_mapper from ..dependencies.authentication import get_current_user_id, get_product_name +from ..dependencies.job_service import get_job_service from ..dependencies.services import get_api_client from ..dependencies.webserver_http import AuthSession, get_webserver_session from ..dependencies.webserver_rpc import ( @@ -96,7 +97,7 @@ async def create_solver_job( inputs: JobInputs, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], - webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], + job_service: Annotated[JobService, Depends(get_job_service)], wb_api_rpc: Annotated[WbApiRpcClient, Depends(get_wb_api_rpc_client)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], @@ -116,8 +117,7 @@ async def create_solver_job( version=version, product_name=product_name, ) - job, project = await create_job( - webserver_api=webserver_api, + job, project = await job_service.create_job( solver_or_program=solver, inputs=inputs, url_for=url_for, diff --git a/services/api-server/src/simcore_service_api_server/core/application.py b/services/api-server/src/simcore_service_api_server/core/application.py index cde8280313ac..818b9cd4e6c3 100644 --- a/services/api-server/src/simcore_service_api_server/core/application.py +++ b/services/api-server/src/simcore_service_api_server/core/application.py @@ -10,6 +10,8 @@ from .. import exceptions from .._meta import API_VERSION, API_VTAG, APP_NAME +from .._service_job import setup as setup_job_service +from .._service_programs import setup as setup_program_service from ..api.root import create_router from ..api.routes.health import router as health_router from ..services_http import catalog, director_v2, storage, webserver @@ -82,6 +84,8 @@ def init_app(settings: ApplicationSettings | None = None) -> FastAPI: app.state.settings = settings setup_rabbitmq(app) + setup_program_service(app) + setup_job_service(app) if settings.API_SERVER_TRACING: initialize_tracing(app, settings.API_SERVER_TRACING, APP_NAME) diff --git a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py index c62dcb90c211..c1fe9461c884 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py +++ b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py @@ -5,7 +5,6 @@ from servicelib.rabbitmq._client_rpc import RabbitMQRPCClient from settings_library.rabbit import RabbitSettings -from .._service_programs import setup as setup_program_service from ..api.dependencies.rabbitmq import get_rabbitmq_rpc_client from ..core.health_checker import ApiServerHealthChecker from ..services_http.log_streaming import LogDistributor @@ -44,9 +43,6 @@ async def _on_startup() -> None: resource_usage_tracker.setup(app, get_rabbitmq_rpc_client(app)) wb_api_server.setup(app, get_rabbitmq_rpc_client(app)) - # setup dependent services layers - setup_program_service(app) - async def _on_shutdown() -> None: if app.state.health_checker: await app.state.health_checker.teardown() From 03c5eb3c676751ff5173191506ce5e5ebe263a67 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 11:56:53 +0200 Subject: [PATCH 07/28] start adding test for creating job --- .../tests/unit/test_api_programs.py | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index 9d4523107d00..968387e0bcff 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -2,13 +2,21 @@ import pytest from fastapi import FastAPI, status from httpx import AsyncClient +from models_library.projects import ProjectID +from models_library.projects_nodes_io import NodeID from models_library.users import UserID from pytest_mock import MockerFixture, MockType +from pytest_simcore.helpers.httpx_calls_capture_models import ( + CreateRespxMockCallback, +) from simcore_service_api_server._meta import API_VTAG +from simcore_service_api_server._service_job import JobService from simcore_service_api_server._service_programs import ProgramService +from simcore_service_api_server.api.dependencies.job_service import get_job_service from simcore_service_api_server.api.dependencies.program_service import ( get_program_service, ) +from simcore_service_api_server.models.schemas.jobs import Job from simcore_service_api_server.models.schemas.programs import Program from simcore_service_api_server.services_rpc.catalog import CatalogService @@ -25,7 +33,22 @@ def _get_program_service(): app.dependency_overrides.pop(get_program_service) -@pytest.mark.asyncio +@pytest.fixture +def mock_job_service( + mocker: MockerFixture, + app: FastAPI, + mocked_webserver_rest_api_base, + create_respx_mock_from_capture: CreateRespxMockCallback, +): + def _get_job_service(): + job_service = JobService(_webserver_api=app.state.webserver_api) + return job_service + + app.dependency_overrides[get_job_service] = _get_job_service + yield + app.dependency_overrides.pop(get_job_service) + + async def test_get_program_release( auth: httpx.BasicAuth, client: AsyncClient, @@ -47,3 +70,34 @@ async def test_get_program_release( program = Program.model_validate(response.json()) assert program.id == program_key assert program.version == version + + +async def test_create_program_job( + auth: httpx.BasicAuth, + client: AsyncClient, + mocked_rpc_catalog_service_api: dict[str, MockType], + mocker: MockerFixture, + mock_program_service: None, + mock_job_service: None, + user_id: UserID, +): + # Arrange + program_key = "simcore/services/dynamic/my_program" + version = "1.0.0" + headers = { + "X-Simcore-Parent-Project-Uuid": str(ProjectID("project-uuid")), + "X-Simcore-Parent-Node-Id": str(NodeID("node-id")), + } + + # Act + response = await client.post( + f"/{API_VTAG}/programs/{program_key}/releases/{version}/jobs", + headers=headers, + auth=auth, + ) + + # Assert + assert response.status_code == status.HTTP_201_CREATED + job = Job.model_validate(response.json()) + assert job.id == "job-id" + assert job.url == "http://testserver/v0/jobs/job-id" From 0cf0dd11a2f764e1c39786ad73051792f84b2b1d Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 13:30:08 +0200 Subject: [PATCH 08/28] minor changes --- services/api-server/tests/unit/test_api_programs.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index 968387e0bcff..a0c81b4f1e69 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -2,8 +2,6 @@ import pytest from fastapi import FastAPI, status from httpx import AsyncClient -from models_library.projects import ProjectID -from models_library.projects_nodes_io import NodeID from models_library.users import UserID from pytest_mock import MockerFixture, MockType from pytest_simcore.helpers.httpx_calls_capture_models import ( @@ -84,15 +82,15 @@ async def test_create_program_job( # Arrange program_key = "simcore/services/dynamic/my_program" version = "1.0.0" - headers = { - "X-Simcore-Parent-Project-Uuid": str(ProjectID("project-uuid")), - "X-Simcore-Parent-Node-Id": str(NodeID("node-id")), - } + # headers = { + # "X-Simcore-Parent-Project-Uuid": str(ProjectID("project-uuid")), + # "X-Simcore-Parent-Node-Id": str(NodeID("node-id")), + # } # Act response = await client.post( f"/{API_VTAG}/programs/{program_key}/releases/{version}/jobs", - headers=headers, + # headers=headers, auth=auth, ) From d94caa787e0422d4c58c141ff0be1b8ce5e4fd38 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 15:54:05 +0200 Subject: [PATCH 09/28] add legacy tasks to webserver --- .../web-server/_long_running_tasks_legacy.py | 61 ++++++++++++++++ api/specs/web-server/openapi.py | 1 + .../api/v0/openapi.yaml | 71 +++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 api/specs/web-server/_long_running_tasks_legacy.py diff --git a/api/specs/web-server/_long_running_tasks_legacy.py b/api/specs/web-server/_long_running_tasks_legacy.py new file mode 100644 index 000000000000..fcbe3508c4dc --- /dev/null +++ b/api/specs/web-server/_long_running_tasks_legacy.py @@ -0,0 +1,61 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + + +from typing import Annotated + +from fastapi import APIRouter, Depends, status +from models_library.generics import Envelope +from servicelib.aiohttp.long_running_tasks._routes import _PathParam +from servicelib.long_running_tasks._models import TaskGet, TaskStatus +from simcore_service_webserver._meta import API_VTAG + +router = APIRouter( + prefix=f"/{API_VTAG}/tasks-legacy", + tags=[ + "long-running-tasks-legacy", + ], +) + + +@router.get( + "", + response_model=Envelope[list[TaskGet]], + name="list_tasks", + description="Lists all long running tasks", +) +def list_tasks(): ... + + +@router.get( + "/{task_id}", + response_model=Envelope[TaskStatus], + name="get_task_status", + description="Retrieves the status of a task", +) +def get_task_status( + _path_params: Annotated[_PathParam, Depends()], +): ... + + +@router.delete( + "/{task_id}", + name="cancel_and_delete_task", + description="Cancels and deletes a task", + status_code=status.HTTP_204_NO_CONTENT, +) +def cancel_and_delete_task( + _path_params: Annotated[_PathParam, Depends()], +): ... + + +@router.get( + "/{task_id}/result", + name="get_task_result", + description="Retrieves the result of a task", +) +def get_task_result( + _path_params: Annotated[_PathParam, Depends()], +): ... diff --git a/api/specs/web-server/openapi.py b/api/specs/web-server/openapi.py index b6fdf69a8e4e..bb32e30f0520 100644 --- a/api/specs/web-server/openapi.py +++ b/api/specs/web-server/openapi.py @@ -36,6 +36,7 @@ "_exporter", "_folders", "_long_running_tasks", + "_long_running_tasks_legacy", "_licensed_items", "_licensed_items_purchases", "_licensed_items_checkouts", diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 615ad40f071c..1f5fbeed12b3 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -3125,6 +3125,77 @@ paths: schema: $ref: '#/components/schemas/EnvelopedError' description: Internal Server Error + /v0/tasks-legacy: + get: + tags: + - long-running-tasks-legacy + summary: List Tasks + description: Lists all long running tasks + operationId: list_tasks + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/Envelope_list_TaskGet__' + /v0/tasks-legacy/{task_id}: + get: + tags: + - long-running-tasks-legacy + summary: Get Task Status + description: Retrieves the status of a task + operationId: get_task_status + parameters: + - name: task_id + in: path + required: true + schema: + type: string + title: Task Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/Envelope_TaskStatus_' + delete: + tags: + - long-running-tasks-legacy + summary: Cancel And Delete Task + description: Cancels and deletes a task + operationId: cancel_and_delete_task + parameters: + - name: task_id + in: path + required: true + schema: + type: string + title: Task Id + responses: + '204': + description: Successful Response + /v0/tasks-legacy/{task_id}/result: + get: + tags: + - long-running-tasks-legacy + summary: Get Task Result + description: Retrieves the result of a task + operationId: get_task_result + parameters: + - name: task_id + in: path + required: true + schema: + type: string + title: Task Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} /v0/catalog/licensed-items: get: tags: From b744dd0fc2660a420e4aedb3a360cb10b4924787 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 15:57:51 +0200 Subject: [PATCH 10/28] mock webserver part of job creation test --- .../helpers/httpx_calls_capture_parameters.py | 4 +- services/api-server/openapi.json | 2 +- .../mocks/create_program_job_success.json | 196 ++++++++++++++++++ services/api-server/tests/unit/conftest.py | 2 +- .../tests/unit/test_api_programs.py | 75 ++++--- 5 files changed, 248 insertions(+), 31 deletions(-) create mode 100644 services/api-server/tests/mocks/create_program_job_success.json diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/httpx_calls_capture_parameters.py b/packages/pytest-simcore/src/pytest_simcore/helpers/httpx_calls_capture_parameters.py index 3e44b62c3b86..a58544a59f09 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/httpx_calls_capture_parameters.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/httpx_calls_capture_parameters.py @@ -7,7 +7,9 @@ class CapturedParameterSchema(BaseModel): title: str | None = None - type_: Literal["str", "int", "float", "bool"] | None = Field(None, alias="type") + type_: Literal["str", "int", "float", "bool", "null"] | None = Field( + None, alias="type" + ) pattern: str | None = None format_: Literal["uuid"] | None = Field(None, alias="format") exclusiveMinimum: bool | None = None diff --git a/services/api-server/openapi.json b/services/api-server/openapi.json index dfafb2a25f70..5da58c97073f 100644 --- a/services/api-server/openapi.json +++ b/services/api-server/openapi.json @@ -1262,7 +1262,7 @@ "programs" ], "summary": "List Programs", - "description": "Lists all available solvers (latest version)\n\nSEE get_solvers_page for paginated version of this function", + "description": "Lists all available solvers (latest version)\n\nDEPRECATION: This implementation and returned values are deprecated", "operationId": "list_programs", "responses": { "200": { diff --git a/services/api-server/tests/mocks/create_program_job_success.json b/services/api-server/tests/mocks/create_program_job_success.json new file mode 100644 index 000000000000..5e1d2b3a5c0a --- /dev/null +++ b/services/api-server/tests/mocks/create_program_job_success.json @@ -0,0 +1,196 @@ +[ + { + "name": "POST /projects", + "description": "", + "method": "POST", + "host": "webserver", + "path": { + "path": "/v0/projects", + "path_parameters": [] + }, + "query": "hidden=false", + "request_payload": { + "uuid": "a6677890-356b-4113-b26e-b77a748427f4", + "name": "programs/simcore%2Fservices%2Fdynamic%2Felectrode-selector/releases/2.1.3/jobs/a6677890-356b-4113-b26e-b77a748427f4", + "description": "Study associated to solver job:\n{\n \"id\": \"a6677890-356b-4113-b26e-b77a748427f4\",\n \"name\": \"programs/simcore%2Fservices%2Fdynamic%2Felectrode-selector/releases/2.1.3/jobs/a6677890-356b-4113-b26e-b77a748427f4\",\n \"inputs_checksum\": \"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\",\n \"created_at\": \"2025-04-15T13:14:44.594564Z\"\n}", + "thumbnail": "https://via.placeholder.com/170x120.png", + "workbench": { + "f298a7d7-900d-533d-9385-89b39ea023fe": { + "key": "simcore/services/dynamic/electrode-selector", + "version": "2.1.3", + "label": "sleeper", + "progress": null, + "thumbnail": null, + "runHash": null, + "inputs": {}, + "inputsRequired": [], + "inputsUnits": {}, + "inputAccess": null, + "inputNodes": [], + "outputs": {}, + "outputNode": null, + "outputNodes": null, + "parent": null, + "position": null, + "state": { + "modified": true, + "dependencies": [], + "currentStatus": "NOT_STARTED", + "progress": 0.0 + }, + "bootOptions": null + } + }, + "accessRights": {}, + "tags": [], + "classifiers": [], + "ui": { + "icon": null, + "workbench": { + "f298a7d7-900d-533d-9385-89b39ea023fe": { + "position": { + "x": 633, + "y": 229 + }, + "marker": null + } + }, + "slideshow": {}, + "currentNodeId": "f298a7d7-900d-533d-9385-89b39ea023fe", + "annotations": {} + }, + "workspaceId": null, + "folderId": null + }, + "response_body": { + "data": { + "task_id": "POST%20%2Fv0%2Fprojects%3Fhidden%3Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8", + "task_name": "POST /v0/projects?hidden=false", + "status_href": "http://webserver:8080/v0/tasks-legacy/POST%2520%252Fv0%252Fprojects%253Fhidden%253Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8", + "result_href": "http://webserver:8080/v0/tasks-legacy/POST%2520%252Fv0%252Fprojects%253Fhidden%253Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8/result", + "abort_href": "http://webserver:8080/v0/tasks-legacy/POST%2520%252Fv0%252Fprojects%253Fhidden%253Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8" + } + }, + "status_code": 202 + }, + { + "name": "GET http://webserver:8080/v0/tasks-legacy/POST%2520%252Fv0%252Fprojects%253Fhidden%253Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8", + "description": "", + "method": "GET", + "host": "webserver", + "path": { + "path": "/v0/tasks-legacy/{task_id}", + "path_parameters": [ + { + "in": "path", + "name": "task_id", + "required": true, + "schema": { + "title": "Task Id", + "type": "str" + }, + "response_value": "tasks-legacy" + } + ] + }, + "response_body": { + "data": { + "task_progress": { + "task_id": "POST%20%2Fv0%2Fprojects%3Fhidden%3Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8", + "message": "finished", + "percent": 1.0 + }, + "done": true, + "started": "2025-04-15T13:14:44.617066" + } + } + }, + { + "name": "GET http://webserver:8080/v0/tasks-legacy/POST%2520%252Fv0%252Fprojects%253Fhidden%253Dfalse.24ca757c-9edb-4017-b0e2-e6f7dcc447c8/result", + "description": "", + "method": "GET", + "host": "webserver", + "path": { + "path": "/v0/tasks-legacy/{task_id}/result", + "path_parameters": [ + { + "in": "path", + "name": "task_id", + "required": true, + "schema": { + "title": "Task Id", + "type": "str" + }, + "response_value": "tasks-legacy" + } + ] + }, + "response_body": { + "data": { + "uuid": "a6677890-356b-4113-b26e-b77a748427f4", + "name": "programs/simcore%2Fservices%2Fdynamic%2Felectrode-selector/releases/2.1.3/jobs/a6677890-356b-4113-b26e-b77a748427f4", + "description": "Study associated to solver job:\n{\n \"id\": \"a6677890-356b-4113-b26e-b77a748427f4\",\n \"name\": \"programs/simcore%2Fservices%2Fdynamic%2Felectrode-selector/releases/2.1.3/jobs/a6677890-356b-4113-b26e-b77a748427f4\",\n \"inputs_checksum\": \"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855\",\n \"created_at\": \"2025-04-15T13:14:44.594564Z\"\n}", + "thumbnail": "https://via.placeholder.com/170x120.png", + "workbench": { + "f298a7d7-900d-533d-9385-89b39ea023fe": { + "key": "simcore/services/dynamic/electrode-selector", + "version": "2.1.3", + "label": "sleeper", + "inputs": {}, + "inputsRequired": [], + "inputsUnits": {}, + "inputNodes": [], + "outputs": {}, + "state": { + "modified": true, + "dependencies": [], + "currentStatus": "NOT_STARTED", + "progress": 0.0 + } + } + }, + "prjOwner": "bisgaard@itis.swiss", + "accessRights": { + "4": { + "read": true, + "write": true, + "delete": true + } + }, + "creationDate": "2025-04-15T13:14:44.636Z", + "lastChangeDate": "2025-04-15T13:14:46.704Z", + "state": { + "locked": { + "value": false, + "status": "CLOSED" + }, + "state": { + "value": "UNKNOWN" + } + }, + "trashedAt": null, + "trashedBy": null, + "tags": [], + "classifiers": [], + "quality": {}, + "ui": { + "workbench": { + "f298a7d7-900d-533d-9385-89b39ea023fe": { + "position": { + "x": 633, + "y": 229 + } + } + }, + "slideshow": {}, + "currentNodeId": "f298a7d7-900d-533d-9385-89b39ea023fe", + "annotations": {} + }, + "dev": {}, + "workspaceId": null, + "folderId": null + } + }, + "status_code": 201 + } +] diff --git a/services/api-server/tests/unit/conftest.py b/services/api-server/tests/unit/conftest.py index 7396f582dcc2..0fd175cf3bd9 100644 --- a/services/api-server/tests/unit/conftest.py +++ b/services/api-server/tests/unit/conftest.py @@ -536,7 +536,7 @@ def _patch(href): return data.status_href, data.result_href return mocker.patch( - "simcore_service_api_server.services.webserver._get_lrt_urls", + "simcore_service_api_server.services_http.webserver._get_lrt_urls", side_effect=_get_lrt_urls, ) diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index a0c81b4f1e69..e171eee5a064 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -1,3 +1,8 @@ +import json +from functools import partial +from pathlib import Path +from typing import Any + import httpx import pytest from fastapi import FastAPI, status @@ -6,11 +11,10 @@ from pytest_mock import MockerFixture, MockType from pytest_simcore.helpers.httpx_calls_capture_models import ( CreateRespxMockCallback, + HttpApiCallCaptureModel, ) from simcore_service_api_server._meta import API_VTAG -from simcore_service_api_server._service_job import JobService from simcore_service_api_server._service_programs import ProgramService -from simcore_service_api_server.api.dependencies.job_service import get_job_service from simcore_service_api_server.api.dependencies.program_service import ( get_program_service, ) @@ -31,22 +35,6 @@ def _get_program_service(): app.dependency_overrides.pop(get_program_service) -@pytest.fixture -def mock_job_service( - mocker: MockerFixture, - app: FastAPI, - mocked_webserver_rest_api_base, - create_respx_mock_from_capture: CreateRespxMockCallback, -): - def _get_job_service(): - job_service = JobService(_webserver_api=app.state.webserver_api) - return job_service - - app.dependency_overrides[get_job_service] = _get_job_service - yield - app.dependency_overrides.pop(get_job_service) - - async def test_get_program_release( auth: httpx.BasicAuth, client: AsyncClient, @@ -70,24 +58,57 @@ async def test_get_program_release( assert program.version == version +@pytest.mark.parametrize("capture_name", ["create_program_job_success.json"]) async def test_create_program_job( auth: httpx.BasicAuth, client: AsyncClient, + mocked_webserver_rest_api_base, mocked_rpc_catalog_service_api: dict[str, MockType], + create_respx_mock_from_capture: CreateRespxMockCallback, mocker: MockerFixture, mock_program_service: None, - mock_job_service: None, user_id: UserID, + capture_name: str, + project_tests_dir: Path, ): + + def _side_effect( + server_state: dict, + request: httpx.Request, + kwargs: dict[str, Any], + capture: HttpApiCallCaptureModel, + ) -> dict[str, Any]: + + response_body = capture.response_body + + # first call defines the project uuid + if server_state.get("project_uuid") is None: + _project_uuid = json.loads(request.content.decode("utf-8")).get("uuid") + assert _project_uuid + server_state["project_uuid"] = _project_uuid + + if request.url.path.endswith("/result"): + capture_uuid = response_body["data"]["uuid"] + response_body["data"]["uuid"] = server_state["project_uuid"] + response_body["data"]["name"] = response_body["data"]["name"].replace( + capture_uuid, server_state["project_uuid"] + ) + assert isinstance(response_body, dict) + return response_body + + # simulate server state + _server_state = dict() + + create_respx_mock_from_capture( + respx_mocks=[mocked_webserver_rest_api_base], + capture_path=project_tests_dir / "mocks" / capture_name, + side_effects_callbacks=3 * [partial(_side_effect, _server_state)], + ) + # Arrange - program_key = "simcore/services/dynamic/my_program" - version = "1.0.0" - # headers = { - # "X-Simcore-Parent-Project-Uuid": str(ProjectID("project-uuid")), - # "X-Simcore-Parent-Node-Id": str(NodeID("node-id")), - # } + program_key = "simcore/services/dynamic/electrode-selector" + version = "2.1.3" - # Act response = await client.post( f"/{API_VTAG}/programs/{program_key}/releases/{version}/jobs", # headers=headers, @@ -97,5 +118,3 @@ async def test_create_program_job( # Assert assert response.status_code == status.HTTP_201_CREATED job = Job.model_validate(response.json()) - assert job.id == "job-id" - assert job.url == "http://testserver/v0/jobs/job-id" From 5c06e9c1594b083aa89c5d9d582162bddc56ae6b Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Tue, 15 Apr 2025 16:24:18 +0200 Subject: [PATCH 11/28] make create job test pass --- services/api-server/tests/unit/test_api_programs.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index e171eee5a064..8823fb61792e 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -7,6 +7,7 @@ import pytest from fastapi import FastAPI, status from httpx import AsyncClient +from models_library.api_schemas_storage.storage_schemas import FileUploadSchema from models_library.users import UserID from pytest_mock import MockerFixture, MockType from pytest_simcore.helpers.httpx_calls_capture_models import ( @@ -72,6 +73,17 @@ async def test_create_program_job( project_tests_dir: Path, ): + mocker.patch( + "simcore_service_api_server.api.routes.programs.get_upload_links_from_s3", + return_value=( + None, + FileUploadSchema.model_validate( + next(iter(FileUploadSchema.model_json_schema()["examples"])) + ), + ), + ) + mocker.patch("simcore_service_api_server.api.routes.programs.complete_file_upload") + def _side_effect( server_state: dict, request: httpx.Request, From 09c95071b92499236041fb584d267de8f3162950 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 09:25:56 +0200 Subject: [PATCH 12/28] use dependency indenction in class constructor --- .../_service_programs.py | 21 +++++++------------ .../api/dependencies/catalog_service.py | 13 ------------ .../api/dependencies/program_service.py | 17 --------------- .../api/routes/programs.py | 5 ++--- .../core/application.py | 2 -- .../services_http/rabbitmq.py | 3 +-- .../services_rpc/catalog.py | 17 ++++++++------- .../tests/unit/test_api_programs.py | 9 +++----- .../tests/unit/test_services_catalog.py | 2 +- 9 files changed, 24 insertions(+), 65 deletions(-) delete mode 100644 services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py delete mode 100644 services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py index 8dd89133fc8b..1119d2104033 100644 --- a/services/api-server/src/simcore_service_api_server/_service_programs.py +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -1,6 +1,6 @@ -from dataclasses import dataclass +from typing import Annotated -from fastapi import FastAPI +from fastapi import Depends from models_library.basic_types import VersionStr from models_library.services_enums import ServiceType from servicelib.fastapi.app_state import SingletonInAppStateMixin @@ -9,15 +9,14 @@ from .services_rpc.catalog import CatalogService -@dataclass class ProgramService(SingletonInAppStateMixin): app_state_name = "ProgramService" - _catalog_service: CatalogService | None = None + _catalog_service: CatalogService - @property - def catalog_service(self) -> CatalogService: - assert self._catalog_service, "ProgramService not initialized" # nosec - return self._catalog_service + def __init__( + self, _catalog_service: Annotated[CatalogService, Depends(CatalogService)] + ): + self._catalog_service = _catalog_service async def get_program( self, @@ -27,7 +26,7 @@ async def get_program( version: VersionStr, product_name: str, ) -> Program: - service = await self.catalog_service.get( + service = await self._catalog_service.get( user_id=user_id, service_key=name, service_version=version, @@ -36,7 +35,3 @@ async def get_program( assert service.service_type == ServiceType.DYNAMIC # nosec return Program.create_from_service(service) - - -def setup(app: FastAPI): - ProgramService().set_to_app_state(app=app) diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py deleted file mode 100644 index 2f07065b1ce3..000000000000 --- a/services/api-server/src/simcore_service_api_server/api/dependencies/catalog_service.py +++ /dev/null @@ -1,13 +0,0 @@ -from typing import Annotated - -from fastapi import Depends, FastAPI -from servicelib.fastapi.dependencies import get_app -from simcore_service_api_server.services_rpc.catalog import CatalogService - - -async def get_catalog_service( - app: Annotated[FastAPI, Depends(get_app)], -) -> CatalogService: - catalog_service = CatalogService.get_from_app_state(app=app) - assert isinstance(catalog_service, CatalogService) - return catalog_service diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py deleted file mode 100644 index 78192dd20575..000000000000 --- a/services/api-server/src/simcore_service_api_server/api/dependencies/program_service.py +++ /dev/null @@ -1,17 +0,0 @@ -from typing import Annotated - -from fastapi import Depends, FastAPI -from servicelib.fastapi.dependencies import get_app - -from ..._service_programs import ProgramService -from ...services_rpc.catalog import CatalogService -from .catalog_service import get_catalog_service - - -async def get_program_service( - app: Annotated[FastAPI, Depends(get_app)], - catalog_service: Annotated[CatalogService, Depends(get_catalog_service)], -) -> ProgramService: - _program_service = ProgramService.get_from_app_state(app=app) - _program_service._catalog_service = catalog_service - return _program_service diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index ad74236e4364..43484eb11a47 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -20,7 +20,6 @@ from ..._service_job import JobService from ..._service_programs import ProgramService -from ...api.dependencies.program_service import get_program_service from ...models.basic_types import VersionStr from ...models.schemas.jobs import Job, JobInputs from ...models.schemas.programs import Program, ProgramKeyId @@ -70,7 +69,7 @@ async def get_program_release( program_key: ProgramKeyId, version: VersionStr, user_id: Annotated[int, Depends(get_current_user_id)], - program_service: Annotated[ProgramService, Depends(get_program_service)], + program_service: Annotated[ProgramService, Depends(ProgramService)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ) -> Program: @@ -109,7 +108,7 @@ async def create_program_job( program_key: ProgramKeyId, version: VersionStr, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], - program_service: Annotated[ProgramService, Depends(get_program_service)], + program_service: Annotated[ProgramService, Depends(ProgramService)], job_service: Annotated[JobService, Depends(get_job_service)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], diff --git a/services/api-server/src/simcore_service_api_server/core/application.py b/services/api-server/src/simcore_service_api_server/core/application.py index 818b9cd4e6c3..c6cbee27fea8 100644 --- a/services/api-server/src/simcore_service_api_server/core/application.py +++ b/services/api-server/src/simcore_service_api_server/core/application.py @@ -11,7 +11,6 @@ from .. import exceptions from .._meta import API_VERSION, API_VTAG, APP_NAME from .._service_job import setup as setup_job_service -from .._service_programs import setup as setup_program_service from ..api.root import create_router from ..api.routes.health import router as health_router from ..services_http import catalog, director_v2, storage, webserver @@ -84,7 +83,6 @@ def init_app(settings: ApplicationSettings | None = None) -> FastAPI: app.state.settings = settings setup_rabbitmq(app) - setup_program_service(app) setup_job_service(app) if settings.API_SERVER_TRACING: diff --git a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py index c1fe9461c884..3466403d590d 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py +++ b/services/api-server/src/simcore_service_api_server/services_http/rabbitmq.py @@ -8,7 +8,7 @@ from ..api.dependencies.rabbitmq import get_rabbitmq_rpc_client from ..core.health_checker import ApiServerHealthChecker from ..services_http.log_streaming import LogDistributor -from ..services_rpc import catalog, resource_usage_tracker, wb_api_server +from ..services_rpc import resource_usage_tracker, wb_api_server _logger = logging.getLogger(__name__) @@ -39,7 +39,6 @@ async def _on_startup() -> None: app.state.settings.API_SERVER_HEALTH_CHECK_TASK_PERIOD_SECONDS ) # setup rpc clients - catalog.setup(app, get_rabbitmq_rpc_client(app)) resource_usage_tracker.setup(app, get_rabbitmq_rpc_client(app)) wb_api_server.setup(app, get_rabbitmq_rpc_client(app)) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index d2da9bbd8bd3..a8040c12c719 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -1,6 +1,6 @@ -from dataclasses import dataclass +from typing import Annotated -from fastapi import FastAPI +from fastapi import Depends from models_library.api_schemas_catalog.services import LatestServiceGet, ServiceGetV2 from models_library.products import ProductName from models_library.rest_pagination import ( @@ -16,12 +16,18 @@ from servicelib.rabbitmq import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc +from ..api.dependencies.rabbitmq import get_rabbitmq_rpc_client + -@dataclass class CatalogService(SingletonInAppStateMixin): app_state_name = "CatalogService" _client: RabbitMQRPCClient + def __init__( + self, client: Annotated[RabbitMQRPCClient, Depends(get_rabbitmq_rpc_client)] + ): + self._client = client + async def list_latest_releases( self, *, @@ -88,8 +94,3 @@ async def get( service_key=service_key, service_version=service_version, ) - - -def setup(app: FastAPI, rabbitmq_rmp_client: RabbitMQRPCClient): - catalog_service = CatalogService(_client=rabbitmq_rmp_client) - catalog_service.set_to_app_state(app=app) diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index 8823fb61792e..e8237d43b8cf 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -16,9 +16,6 @@ ) from simcore_service_api_server._meta import API_VTAG from simcore_service_api_server._service_programs import ProgramService -from simcore_service_api_server.api.dependencies.program_service import ( - get_program_service, -) from simcore_service_api_server.models.schemas.jobs import Job from simcore_service_api_server.models.schemas.programs import Program from simcore_service_api_server.services_rpc.catalog import CatalogService @@ -28,12 +25,12 @@ def mock_program_service(mocker: MockerFixture, app: FastAPI): def _get_program_service(): - catalog_service = CatalogService(_client=mocker.MagicMock()) + catalog_service = CatalogService(client=mocker.MagicMock()) return ProgramService(_catalog_service=catalog_service) - app.dependency_overrides[get_program_service] = _get_program_service + app.dependency_overrides[ProgramService] = _get_program_service yield - app.dependency_overrides.pop(get_program_service) + app.dependency_overrides.pop(ProgramService) async def test_get_program_release( diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index ae2cb4145e65..77b421b30719 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -41,7 +41,7 @@ async def test_catalog_service_read_solvers( mocker: MockerFixture, mocked_rpc_catalog_service_api: dict[str, MockType], ): - catalog_service = CatalogService(_client=mocker.MagicMock()) + catalog_service = CatalogService(client=mocker.MagicMock()) catalog_service.set_to_app_state(app=FastAPI()) # Step 1: List latest releases in a page From ec6a680a45cda11141e35ddaf7da41ce9cfe7d91 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 09:34:02 +0200 Subject: [PATCH 13/28] use dependency injection in JobService constructor --- .../_service_job.py | 33 +++++++++---------- .../api/dependencies/job_service.py | 17 ---------- .../api/routes/programs.py | 3 +- .../api/routes/solvers_jobs.py | 3 +- .../core/application.py | 2 -- 5 files changed, 17 insertions(+), 41 deletions(-) delete mode 100644 services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py diff --git a/services/api-server/src/simcore_service_api_server/_service_job.py b/services/api-server/src/simcore_service_api_server/_service_job.py index fe378a9d8dcd..d44827370076 100644 --- a/services/api-server/src/simcore_service_api_server/_service_job.py +++ b/services/api-server/src/simcore_service_api_server/_service_job.py @@ -1,34 +1,35 @@ import logging from collections.abc import Callable -from dataclasses import dataclass +from typing import Annotated -from fastapi import FastAPI +from fastapi import Depends from models_library.api_schemas_webserver.projects import ProjectCreateNew, ProjectGet from models_library.projects import ProjectID from models_library.projects_nodes_io import NodeID from pydantic import HttpUrl from servicelib.fastapi.app_state import SingletonInAppStateMixin -from simcore_service_api_server.models.schemas.jobs import Job, JobInputs -from simcore_service_api_server.models.schemas.programs import Program -from simcore_service_api_server.models.schemas.solvers import Solver -from simcore_service_api_server.services_http.solver_job_models_converters import ( + +from .api.dependencies.webserver_http import get_webserver_session +from .models.schemas.jobs import Job, JobInputs +from .models.schemas.programs import Program +from .models.schemas.solvers import Solver +from .services_http.solver_job_models_converters import ( create_job_from_project, create_new_project_for_job, ) -from simcore_service_api_server.services_http.webserver import AuthSession +from .services_http.webserver import AuthSession _logger = logging.getLogger(__name__) -@dataclass() class JobService(SingletonInAppStateMixin): app_state_name = "JobService" - _webserver_api: AuthSession | None = None + _webserver_api: AuthSession - @property - def webserver_api(self) -> AuthSession: - assert self._webserver_api, "JobService not initialized" # nosec - return self._webserver_api + def __init__( + self, webserver_api: Annotated[AuthSession, Depends(get_webserver_session)] + ): + self._webserver_api = webserver_api async def create_job( self, @@ -49,7 +50,7 @@ async def create_job( project_in: ProjectCreateNew = create_new_project_for_job( solver_or_program, pre_job, inputs ) - new_project: ProjectGet = await self.webserver_api.create_project( + new_project: ProjectGet = await self._webserver_api.create_project( project_in, is_hidden=hidden, parent_project_uuid=parent_project_uuid, @@ -69,7 +70,3 @@ async def create_job( job_id=job.id, ) return job, new_project - - -def setup(app: FastAPI): - JobService().set_to_app_state(app=app) diff --git a/services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py b/services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py deleted file mode 100644 index 37fb41be6185..000000000000 --- a/services/api-server/src/simcore_service_api_server/api/dependencies/job_service.py +++ /dev/null @@ -1,17 +0,0 @@ -from typing import Annotated - -from fastapi import Depends, FastAPI -from servicelib.fastapi.dependencies import get_app -from simcore_service_api_server._service_job import JobService -from simcore_service_api_server.services_http.webserver import AuthSession - -from .webserver_http import get_webserver_session - - -async def get_job_service( - app: Annotated[FastAPI, Depends(get_app)], - webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], -) -> JobService: - job_service = JobService.get_from_app_state(app=app) - job_service._webserver_api = webserver_api - return job_service diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index 43484eb11a47..886650bfdc0f 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -25,7 +25,6 @@ from ...models.schemas.programs import Program, ProgramKeyId from ...services_http.catalog import CatalogApi from ..dependencies.authentication import get_current_user_id, get_product_name -from ..dependencies.job_service import get_job_service from ..dependencies.services import get_api_client _logger = logging.getLogger(__name__) @@ -109,7 +108,7 @@ async def create_program_job( version: VersionStr, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], program_service: Annotated[ProgramService, Depends(ProgramService)], - job_service: Annotated[JobService, Depends(get_job_service)], + job_service: Annotated[JobService, Depends(JobService)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], x_simcore_parent_project_uuid: Annotated[ProjectID | None, Header()] = None, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py index 9d101c69f61c..819dd740f37e 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py @@ -35,7 +35,6 @@ from ...services_rpc.wb_api_server import WbApiRpcClient from ..dependencies.application import get_reverse_url_mapper from ..dependencies.authentication import get_current_user_id, get_product_name -from ..dependencies.job_service import get_job_service from ..dependencies.services import get_api_client from ..dependencies.webserver_http import AuthSession, get_webserver_session from ..dependencies.webserver_rpc import ( @@ -97,7 +96,7 @@ async def create_solver_job( inputs: JobInputs, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], - job_service: Annotated[JobService, Depends(get_job_service)], + job_service: Annotated[JobService, Depends(JobService)], wb_api_rpc: Annotated[WbApiRpcClient, Depends(get_wb_api_rpc_client)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], diff --git a/services/api-server/src/simcore_service_api_server/core/application.py b/services/api-server/src/simcore_service_api_server/core/application.py index c6cbee27fea8..cde8280313ac 100644 --- a/services/api-server/src/simcore_service_api_server/core/application.py +++ b/services/api-server/src/simcore_service_api_server/core/application.py @@ -10,7 +10,6 @@ from .. import exceptions from .._meta import API_VERSION, API_VTAG, APP_NAME -from .._service_job import setup as setup_job_service from ..api.root import create_router from ..api.routes.health import router as health_router from ..services_http import catalog, director_v2, storage, webserver @@ -83,7 +82,6 @@ def init_app(settings: ApplicationSettings | None = None) -> FastAPI: app.state.settings = settings setup_rabbitmq(app) - setup_job_service(app) if settings.API_SERVER_TRACING: initialize_tracing(app, settings.API_SERVER_TRACING, APP_NAME) From 93b99661af017d0d087342269fe115ff85babf82 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 10:25:39 +0200 Subject: [PATCH 14/28] minor changes --- .../src/simcore_service_api_server/_service_programs.py | 4 +--- .../src/simcore_service_api_server/services_rpc/catalog.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py index 1119d2104033..f822c54dd6ff 100644 --- a/services/api-server/src/simcore_service_api_server/_service_programs.py +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -3,14 +3,12 @@ from fastapi import Depends from models_library.basic_types import VersionStr from models_library.services_enums import ServiceType -from servicelib.fastapi.app_state import SingletonInAppStateMixin from .models.schemas.programs import Program, ProgramKeyId from .services_rpc.catalog import CatalogService -class ProgramService(SingletonInAppStateMixin): - app_state_name = "ProgramService" +class ProgramService: _catalog_service: CatalogService def __init__( diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index a8040c12c719..cf046fe76b63 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -12,15 +12,13 @@ from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID -from servicelib.fastapi.app_state import SingletonInAppStateMixin from servicelib.rabbitmq import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc from ..api.dependencies.rabbitmq import get_rabbitmq_rpc_client -class CatalogService(SingletonInAppStateMixin): - app_state_name = "CatalogService" +class CatalogService: _client: RabbitMQRPCClient def __init__( From aac464c8c094a8cd0af9b49121755c58576f0d10 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 11:37:35 +0200 Subject: [PATCH 15/28] fix tests --- .../_service_solvers.py | 39 +++++++++++++++++++ .../api/routes/solvers.py | 5 ++- .../api/routes/solvers_jobs.py | 6 +-- .../api/routes/solvers_jobs_getters.py | 14 +++---- .../models/schemas/solvers.py | 15 +++++++ .../services_http/catalog.py | 20 +--------- .../tests/unit/_with_db/test_product.py | 21 ++-------- .../test_api_routers_solvers_jobs.py | 18 +-------- .../test_api_routers_solvers_jobs_delete.py | 27 ++++++------- .../test_api_routers_solvers_jobs_metadata.py | 15 +------ .../test_api_routers_solvers_jobs_read.py | 19 ++------- services/api-server/tests/unit/conftest.py | 15 +++++-- .../tests/unit/test_api_programs.py | 18 +-------- .../tests/unit/test_services_catalog.py | 2 +- 14 files changed, 104 insertions(+), 130 deletions(-) create mode 100644 services/api-server/src/simcore_service_api_server/_service_solvers.py diff --git a/services/api-server/src/simcore_service_api_server/_service_solvers.py b/services/api-server/src/simcore_service_api_server/_service_solvers.py new file mode 100644 index 000000000000..d46d1a9b3866 --- /dev/null +++ b/services/api-server/src/simcore_service_api_server/_service_solvers.py @@ -0,0 +1,39 @@ +from typing import Annotated + +from fastapi import Depends +from models_library.basic_types import VersionStr +from models_library.products import ProductName +from models_library.services_enums import ServiceType +from models_library.users import UserID + +from .models.schemas.solvers import Solver, SolverKeyId +from .services_rpc.catalog import CatalogService + + +class SolverService: + _catalog_service: CatalogService + + def __init__( + self, catalog_service: Annotated[CatalogService, Depends(CatalogService)] + ): + self._catalog_service = catalog_service + + async def get_solver( + self, + *, + user_id: UserID, + name: SolverKeyId, + version: VersionStr, + product_name: ProductName, + ) -> Solver: + service = await self._catalog_service.get( + user_id=user_id, + service_key=name, + service_version=version, + product_name=product_name, + ) + assert ( # nosec + service.service_type == ServiceType.COMPUTATIONAL + ), "Expected by SolverName regex" + + return Solver.create_from_service(service) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py index 89b220d2d5fd..2fd92bc0c975 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py @@ -7,6 +7,7 @@ from httpx import HTTPStatusError from pydantic import ValidationError +from ..._service_solvers import SolverService from ...exceptions.service_errors_utils import DEFAULT_BACKEND_SERVICE_STATUS_CODES from ...models.basic_types import VersionStr from ...models.pagination import OnePage, Page, PaginationParams @@ -214,13 +215,13 @@ async def get_solver_release( solver_key: SolverKeyId, version: VersionStr, user_id: Annotated[int, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + solver_service: Annotated[SolverService, Depends(SolverService)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ) -> Solver: """Gets a specific release of a solver""" try: - solver: Solver = await catalog_client.get_solver( + solver: Solver = await solver_service.get_solver( user_id=user_id, name=solver_key, version=version, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py index 819dd740f37e..9e0250ef20e0 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py @@ -13,6 +13,7 @@ from pydantic.types import PositiveInt from ..._service_job import JobService +from ..._service_solvers import SolverService from ...exceptions.backend_errors import ProjectAlreadyStartedError from ...exceptions.service_errors_utils import DEFAULT_BACKEND_SERVICE_STATUS_CODES from ...models.basic_types import VersionStr @@ -26,7 +27,6 @@ JobStatus, ) from ...models.schemas.solvers import Solver, SolverKeyId -from ...services_http.catalog import CatalogApi from ...services_http.director_v2 import DirectorV2Api from ...services_http.jobs import replace_custom_metadata, start_project, stop_project from ...services_http.solver_job_models_converters import ( @@ -95,7 +95,7 @@ async def create_solver_job( version: VersionStr, inputs: JobInputs, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + solver_service: Annotated[SolverService, Depends(SolverService)], job_service: Annotated[JobService, Depends(JobService)], wb_api_rpc: Annotated[WbApiRpcClient, Depends(get_wb_api_rpc_client)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], @@ -110,7 +110,7 @@ async def create_solver_job( """ # ensures user has access to solver - solver = await catalog_client.get_solver( + solver = await solver_service.get_solver( user_id=user_id, name=solver_key, version=version, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py index e156863c90b4..127d5468c2ee 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py @@ -19,6 +19,7 @@ from servicelib.fastapi.requests_decorators import cancel_on_disconnect from servicelib.logging_utils import log_context +from ..._service_solvers import SolverService from ...exceptions.custom_errors import InsufficientCreditsError, MissingWalletError from ...exceptions.service_errors_utils import DEFAULT_BACKEND_SERVICE_STATUS_CODES from ...models.basic_types import LogStreamingResponse, VersionStr @@ -39,7 +40,6 @@ WalletGetWithAvailableCreditsLegacy, ) from ...models.schemas.solvers import SolverKeyId -from ...services_http.catalog import CatalogApi from ...services_http.director_v2 import DirectorV2Api from ...services_http.jobs import ( get_custom_metadata, @@ -123,7 +123,7 @@ async def list_jobs( solver_key: SolverKeyId, version: VersionStr, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + solver_service: Annotated[SolverService, Depends(SolverService)], webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], @@ -134,7 +134,7 @@ async def list_jobs( - SEE `get_jobs_page` for paginated version of this function """ - solver = await catalog_client.get_solver( + solver = await solver_service.get_solver( user_id=user_id, name=solver_key, version=version, @@ -173,7 +173,7 @@ async def get_jobs_page( version: VersionStr, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], page_params: Annotated[PaginationParams, Depends()], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + solver_service: Annotated[SolverService, Depends(SolverService)], webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], @@ -181,7 +181,7 @@ async def get_jobs_page( # NOTE: Different entry to keep backwards compatibility with list_jobs. # Eventually use a header with agent version to switch to new interface - solver = await catalog_client.get_solver( + solver = await solver_service.get_solver( user_id=user_id, name=solver_key, version=version, @@ -217,7 +217,7 @@ async def get_job( user_id: Annotated[PositiveInt, Depends(get_current_user_id)], product_name: Annotated[str, Depends(get_product_name)], webserver_api: Annotated[AuthSession, Depends(get_webserver_session)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + solver_service: Annotated[SolverService, Depends(SolverService)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], ): """Gets job of a given solver""" @@ -225,7 +225,7 @@ async def get_job( "Getting Job '%s'", _compose_job_resource_name(solver_key, version, job_id) ) - solver = await catalog_client.get_solver( + solver = await solver_service.get_solver( user_id=user_id, name=solver_key, version=version, diff --git a/services/api-server/src/simcore_service_api_server/models/schemas/solvers.py b/services/api-server/src/simcore_service_api_server/models/schemas/solvers.py index 6cae1156a7d7..84e0b7007232 100644 --- a/services/api-server/src/simcore_service_api_server/models/schemas/solvers.py +++ b/services/api-server/src/simcore_service_api_server/models/schemas/solvers.py @@ -1,5 +1,6 @@ from typing import Annotated, Any, Literal +from models_library.api_schemas_catalog.services import ServiceGetV2 from models_library.basic_regex import PUBLIC_VARIABLE_NAME_RE from models_library.services import ServiceMetaDataPublished from models_library.services_regex import COMPUTATIONAL_SERVICE_KEY_RE @@ -65,6 +66,20 @@ def create_from_image(cls, image_meta: ServiceMetaDataPublished) -> "Solver": **data, ) + @classmethod + def create_from_service(cls, service: ServiceGetV2) -> "Solver": + data = service.model_dump( + include={"name", "key", "version", "description", "contact"}, + ) + return cls( + id=data.pop("key"), + version=data.pop("version"), + title=data.pop("name"), + url=None, + maintainer=data.pop("contact"), + **data, + ) + @classmethod def compose_resource_name(cls, key: str, version: str) -> str: return compose_resource_name("solvers", key, "releases", version) diff --git a/services/api-server/src/simcore_service_api_server/services_http/catalog.py b/services/api-server/src/simcore_service_api_server/services_http/catalog.py index d9a8b26adb42..26af019697b6 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_http/catalog.py @@ -22,7 +22,7 @@ ) from ..exceptions.service_errors_utils import service_exception_mapper from ..models.basic_types import VersionStr -from ..models.schemas.programs import Program, ProgramKeyId +from ..models.schemas.programs import Program from ..models.schemas.solvers import LATEST_VERSION, Solver, SolverKeyId, SolverPort from ..utils.client_base import BaseServiceClientApi, setup_client_instance @@ -163,24 +163,6 @@ async def get_solver( solver: Solver = service.to_solver() return solver - async def get_program( - self, - *, - user_id: int, - name: ProgramKeyId, - version: VersionStr, - product_name: str, - ) -> Program: - service = await self._get_service( - user_id=user_id, name=name, version=version, product_name=product_name - ) - assert ( # nosec - service.service_type == ServiceType.DYNAMIC - ), "Expected by ProgramName regex" - - program = service.to_program() - return program - @_exception_mapper( http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} ) diff --git a/services/api-server/tests/unit/_with_db/test_product.py b/services/api-server/tests/unit/_with_db/test_product.py index acf209496181..5c83ec557fa4 100644 --- a/services/api-server/tests/unit/_with_db/test_product.py +++ b/services/api-server/tests/unit/_with_db/test_product.py @@ -15,9 +15,9 @@ from fastapi.encoders import jsonable_encoder from models_library.api_schemas_api_server.api_keys import ApiKeyInDB from models_library.generics import Envelope -from models_library.users import UserID from models_library.wallets import WalletStatus from pydantic import PositiveInt +from pytest_mock import MockType from simcore_service_api_server._meta import API_VTAG from simcore_service_api_server.models.schemas.model_adapter import ( WalletGetWithAvailableCreditsLegacy, @@ -80,7 +80,7 @@ def _check_key_product_compatibility(request: httpx.Request, **kwargs): async def test_product_catalog( client: httpx.AsyncClient, - mocked_catalog_rest_api_base: respx.MockRouter, + mocked_rpc_catalog_service_api: dict[str, MockType], create_fake_api_keys: Callable[[PositiveInt], AsyncGenerator[ApiKeyInDB, None]], ) -> None: assert client @@ -88,25 +88,10 @@ async def test_product_catalog( keys: list[ApiKeyInDB] = [key async for key in create_fake_api_keys(2)] assert len({key.product_name for key in keys}) == 2 - def _get_service_side_effect(request: httpx.Request, **kwargs): - assert ( - received_product := request.headers.get("x-simcore-products-name") - ) is not None - assert (user_id := request.url.params.get("user_id")) is not None - assert ( - key := {UserID(key.id_): key for key in keys}.get(UserID(user_id)) - ) is not None - assert key.product_name == received_product - return httpx.Response(status_code=status.HTTP_200_OK) - - respx_mock = mocked_catalog_rest_api_base.get( - r"/v0/services/simcore%2Fservices%2Fcomp%2Fisolve/2.0.24" - ).mock(side_effect=_get_service_side_effect) - for key in keys: await client.get( f"{API_VTAG}/solvers/simcore/services/comp/isolve/releases/2.0.24", auth=httpx.BasicAuth(key.api_key, key.api_secret), ) - assert respx_mock.call_count == len(keys) + assert mocked_rpc_catalog_service_api["get_service"].called diff --git a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs.py b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs.py index a167f2b4603d..eb6bd838a84c 100644 --- a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs.py +++ b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs.py @@ -204,7 +204,7 @@ async def test_run_solver_job( client: httpx.AsyncClient, directorv2_service_openapi_specs: dict[str, Any], catalog_service_openapi_specs: dict[str, Any], - mocked_catalog_rest_api: MockRouter, + mocked_rpc_catalog_service_api: dict[str, MockType], mocked_directorv2_service_api: MockRouter, mocked_webserver_rest_api: MockRouter, mocked_webserver_rpc_api: dict[str, MockType], @@ -319,22 +319,6 @@ async def test_run_solver_job( if "boot-options" in e ) - mocked_catalog_rest_api.get( - # path__regex=r"/services/(?P[\w-]+)/(?P[0-9\.]+)", - path=f"/v0/services/{solver_key}/{solver_version}", - name="get_service_v0_services__service_key___service_version__get", - ).respond( - status.HTTP_200_OK, - json=example - | { - "name": solver_key.split("/")[-1].capitalize(), - "description": solver_key.replace("/", " "), - "key": solver_key, - "version": solver_version, - "type": "computational", - }, - ) - # --------------------------------------------------------------------------------------------------------- resp = await client.get(f"/{API_VTAG}/meta") diff --git a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_delete.py b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_delete.py index c5fad3f6d2ae..23966e15bd1e 100644 --- a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_delete.py +++ b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_delete.py @@ -27,7 +27,6 @@ class MockedBackendApiDict(TypedDict): - catalog: MockRouter | None webserver: MockRouter | None @@ -56,7 +55,7 @@ def _response(request: httpx.Request, project_id: str): name="delete_project", ).mock(side_effect=_response) - return MockedBackendApiDict(webserver=mocked_webserver_rest_api, catalog=None) + return MockedBackendApiDict(webserver=mocked_webserver_rest_api) @pytest.mark.acceptance_test( @@ -88,7 +87,6 @@ async def test_delete_non_existing_solver_job( def mocked_backend_services_apis_for_create_and_delete_solver_job( mocked_webserver_rest_api: MockRouter, mocked_webserver_rpc_api: dict[str, MockType], - mocked_catalog_rest_api: MockRouter, project_tests_dir: Path, ) -> MockedBackendApiDict: mock_name = "on_create_job.json" @@ -98,12 +96,12 @@ def mocked_backend_services_apis_for_create_and_delete_solver_job( Path(project_tests_dir / "mocks" / mock_name).read_text() ) - capture = captures[0] - assert capture.host == "catalog" - assert capture.method == "GET" - mocked_catalog_rest_api.request( - method=capture.method, path=capture.path, name="get_service" # GET service - ).respond(status_code=capture.status_code, json=capture.response_body) + # capture = captures[0] + # assert capture.host == "catalog" + # assert capture.method == "GET" + # mocked_catalog_rest_api.request( + # method=capture.method, path=capture.path, name="get_service" # GET service + # ).respond(status_code=capture.status_code, json=capture.response_body) capture = captures[-1] assert capture.host == "webserver" @@ -115,7 +113,6 @@ def mocked_backend_services_apis_for_create_and_delete_solver_job( ).respond(status_code=capture.status_code, json=capture.response_body) return MockedBackendApiDict( - catalog=mocked_catalog_rest_api, webserver=mocked_webserver_rest_api, ) @@ -128,6 +125,7 @@ async def test_create_and_delete_solver_job( client: httpx.AsyncClient, solver_key: str, solver_version: str, + mocked_rpc_catalog_service_api: dict[str, MockType], mocked_backend_services_apis_for_create_and_delete_solver_job: MockedBackendApiDict, ): # create Job @@ -157,11 +155,9 @@ async def test_create_and_delete_solver_job( assert mock_webserver_router assert mock_webserver_router["delete_project"].called - mock_catalog_router = mocked_backend_services_apis_for_create_and_delete_solver_job[ - "catalog" - ] - assert mock_catalog_router - assert mock_catalog_router["get_service"].called + get_service = mocked_rpc_catalog_service_api["get_service"] + assert get_service + assert get_service.called # NOTE: ideas for further tests # Run job and try to delete while running @@ -179,6 +175,7 @@ async def test_create_job( solver_key: str, solver_version: str, mocked_backend_services_apis_for_create_and_delete_solver_job: MockedBackendApiDict, + mocked_rpc_catalog_service_api: dict[str, MockType], hidden: bool, parent_project_id: UUID | None, parent_node_id: UUID | None, diff --git a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_metadata.py b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_metadata.py index 1693e579fa8b..419492287633 100644 --- a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_metadata.py +++ b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_metadata.py @@ -25,7 +25,6 @@ class MockedBackendApiDict(TypedDict): - catalog: MockRouter | None webserver: MockRouter | None @@ -40,7 +39,7 @@ def _as_path_regex(initial_path: str): def mocked_backend( mocked_webserver_rest_api: MockRouter, mocked_webserver_rpc_api: dict[str, MockType], - mocked_catalog_rest_api: MockRouter, + mocked_rpc_catalog_service_api: dict[str, MockType], project_tests_dir: Path, ) -> MockedBackendApiDict: mock_name = "for_test_get_and_update_job_metadata.json" @@ -54,14 +53,6 @@ def mocked_backend( capture = captures["get_service"] assert capture.host == "catalog" - mocked_catalog_rest_api.request( - method=capture.method, - path=capture.path, - name=capture.name, - ).respond( - status_code=capture.status_code, - json=capture.response_body, - ) for name in ("get_project_metadata", "update_project_metadata", "delete_project"): capture = captures[name] @@ -87,9 +78,7 @@ def mocked_backend( json=capture.response_body, ) - return MockedBackendApiDict( - webserver=mocked_webserver_rest_api, catalog=mocked_catalog_rest_api - ) + return MockedBackendApiDict(webserver=mocked_webserver_rest_api) @pytest.mark.acceptance_test( diff --git a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_read.py b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_read.py index 09fc53194f62..7f3be4e216f8 100644 --- a/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_read.py +++ b/services/api-server/tests/unit/api_solvers/test_api_routers_solvers_jobs_read.py @@ -8,6 +8,7 @@ import httpx import pytest from pydantic import TypeAdapter +from pytest_mock import MockType from pytest_simcore.helpers.httpx_calls_capture_models import HttpApiCallCaptureModel from respx import MockRouter from simcore_service_api_server._meta import API_VTAG @@ -17,14 +18,14 @@ class MockBackendRouters(NamedTuple): - catalog: MockRouter webserver: MockRouter + catalog: dict[str, MockType] @pytest.fixture def mocked_backend( mocked_webserver_rest_api_base: MockRouter, - mocked_catalog_rest_api_base: MockRouter, + mocked_rpc_catalog_service_api: dict[str, MockType], project_tests_dir: Path, ) -> MockBackendRouters: mock_name = "on_list_jobs.json" @@ -32,18 +33,6 @@ def mocked_backend( Path(project_tests_dir / "mocks" / mock_name).read_text() ) - capture = captures[0] - assert capture.host == "catalog" - assert capture.name == "get_service" - mocked_catalog_rest_api_base.request( - method=capture.method, - path=capture.path, - name=capture.name, - ).respond( - status_code=capture.status_code, - json=capture.response_body, - ) - capture = captures[1] assert capture.host == "webserver" assert capture.name == "list_projects" @@ -57,8 +46,8 @@ def mocked_backend( ) return MockBackendRouters( - catalog=mocked_catalog_rest_api_base, webserver=mocked_webserver_rest_api_base, + catalog=mocked_rpc_catalog_service_api, ) diff --git a/services/api-server/tests/unit/conftest.py b/services/api-server/tests/unit/conftest.py index 0fd175cf3bd9..c96fa8d8666a 100644 --- a/services/api-server/tests/unit/conftest.py +++ b/services/api-server/tests/unit/conftest.py @@ -5,7 +5,7 @@ import json import subprocess -from collections.abc import AsyncIterator, Callable, Iterator +from collections.abc import AsyncIterator, Callable, Iterable, Iterator from copy import deepcopy from pathlib import Path from typing import Any @@ -47,6 +47,7 @@ from respx import MockRouter from servicelib.rabbitmq._client_rpc import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc +from simcore_service_api_server.api.dependencies.rabbitmq import get_rabbitmq_rpc_client from simcore_service_api_server.core.application import init_app from simcore_service_api_server.core.settings import ApplicationSettings from simcore_service_api_server.db.repositories.api_keys import UserAndProductTuple @@ -54,13 +55,20 @@ @pytest.fixture -def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType]: +def mocked_rpc_catalog_service_api( + app: FastAPI, mocker: MockerFixture +) -> Iterable[dict[str, MockType]]: """ Mocks the RPC catalog service API for testing purposes. """ + + class MockRabbitMQRPCClient: + pass + + app.dependency_overrides[get_rabbitmq_rpc_client] = lambda: MockRabbitMQRPCClient() side_effects = CatalogRpcSideEffects() - return { + yield { "list_services_paginated": mocker.patch.object( catalog_rpc, "list_services_paginated", @@ -86,6 +94,7 @@ def mocked_rpc_catalog_service_api(mocker: MockerFixture) -> dict[str, MockType] side_effect=side_effects.list_my_service_history_paginated, ), } + app.dependency_overrides.pop(get_rabbitmq_rpc_client) @pytest.fixture diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index e8237d43b8cf..22d369d39797 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -5,7 +5,7 @@ import httpx import pytest -from fastapi import FastAPI, status +from fastapi import status from httpx import AsyncClient from models_library.api_schemas_storage.storage_schemas import FileUploadSchema from models_library.users import UserID @@ -15,22 +15,8 @@ HttpApiCallCaptureModel, ) from simcore_service_api_server._meta import API_VTAG -from simcore_service_api_server._service_programs import ProgramService from simcore_service_api_server.models.schemas.jobs import Job from simcore_service_api_server.models.schemas.programs import Program -from simcore_service_api_server.services_rpc.catalog import CatalogService - - -@pytest.fixture -def mock_program_service(mocker: MockerFixture, app: FastAPI): - - def _get_program_service(): - catalog_service = CatalogService(client=mocker.MagicMock()) - return ProgramService(_catalog_service=catalog_service) - - app.dependency_overrides[ProgramService] = _get_program_service - yield - app.dependency_overrides.pop(ProgramService) async def test_get_program_release( @@ -38,7 +24,6 @@ async def test_get_program_release( client: AsyncClient, mocked_rpc_catalog_service_api: dict[str, MockType], mocker: MockerFixture, - mock_program_service: None, user_id: UserID, ): # Arrange @@ -64,7 +49,6 @@ async def test_create_program_job( mocked_rpc_catalog_service_api: dict[str, MockType], create_respx_mock_from_capture: CreateRespxMockCallback, mocker: MockerFixture, - mock_program_service: None, user_id: UserID, capture_name: str, project_tests_dir: Path, diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index 77b421b30719..40d945023c18 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -36,13 +36,13 @@ def to_solver( async def test_catalog_service_read_solvers( + app: FastAPI, product_name: ProductName, user_id: UserID, mocker: MockerFixture, mocked_rpc_catalog_service_api: dict[str, MockType], ): catalog_service = CatalogService(client=mocker.MagicMock()) - catalog_service.set_to_app_state(app=FastAPI()) # Step 1: List latest releases in a page latest_releases, meta = await catalog_service.list_latest_releases( From e6e6baa38f33b22154565c93d9e3f130bbf46128 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 11:38:28 +0200 Subject: [PATCH 16/28] remove get_solver method from http catalog client --- .../services_http/catalog.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/services_http/catalog.py b/services/api-server/src/simcore_service_api_server/services_http/catalog.py index 26af019697b6..892066d8d769 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_http/catalog.py @@ -145,24 +145,6 @@ async def list_services( services = [service for service in services if predicate(service)] return services - async def get_solver( - self, - *, - user_id: UserID, - name: SolverKeyId, - version: VersionStr, - product_name: ProductName, - ) -> Solver: - service = await self._get_service( - user_id=user_id, name=name, version=version, product_name=product_name - ) - assert ( # nosec - service.service_type == ServiceType.COMPUTATIONAL - ), "Expected by SolverName regex" - - solver: Solver = service.to_solver() - return solver - @_exception_mapper( http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} ) From 65916c4f88f46d9a4ef0d1b6d96df3a910570e25 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 13:43:42 +0200 Subject: [PATCH 17/28] add test for getting latest solver --- .../_service_solvers.py | 35 +++++++++++++++ .../api/routes/solvers.py | 4 +- .../services_http/catalog.py | 43 ------------------- .../api-server/tests/unit/test_api_solvers.py | 23 ++++++++++ 4 files changed, 60 insertions(+), 45 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/_service_solvers.py b/services/api-server/src/simcore_service_api_server/_service_solvers.py index d46d1a9b3866..da85552a5d77 100644 --- a/services/api-server/src/simcore_service_api_server/_service_solvers.py +++ b/services/api-server/src/simcore_service_api_server/_service_solvers.py @@ -1,14 +1,20 @@ from typing import Annotated +from common_library.pagination_tools import iter_pagination_params from fastapi import Depends from models_library.basic_types import VersionStr from models_library.products import ProductName +from models_library.rest_pagination import MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE from models_library.services_enums import ServiceType +from models_library.services_history import ServiceRelease from models_library.users import UserID +from packaging.version import Version from .models.schemas.solvers import Solver, SolverKeyId from .services_rpc.catalog import CatalogService +DEFAULT_PAGINATION_LIMIT = MAXIMUM_NUMBER_OF_ITEMS_PER_PAGE - 1 + class SolverService: _catalog_service: CatalogService @@ -37,3 +43,32 @@ async def get_solver( ), "Expected by SolverName regex" return Solver.create_from_service(service) + + async def get_latest_release( + self, + *, + user_id: int, + solver_key: SolverKeyId, + product_name: str, + ) -> Solver: + service_releases: list[ServiceRelease] = [] + for page_params in iter_pagination_params(limit=DEFAULT_PAGINATION_LIMIT): + releases, page_meta = await self._catalog_service.list_release_history( + user_id=user_id, + service_key=solver_key, + product_name=product_name, + offset=page_params.offset, + limit=page_params.limit, + ) + page_params.total_number_of_items = page_meta.total + service_releases.extend(releases) + + release = sorted(service_releases, key=lambda s: Version(s.version))[-1] + service = await self._catalog_service.get( + user_id=user_id, + service_key=solver_key, + service_version=release.version, + product_name=product_name, + ) + + return Solver.create_from_service(service) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py index 2fd92bc0c975..d67f2cb0d868 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py @@ -136,7 +136,7 @@ async def get_solvers_releases_page( async def get_solver( solver_key: SolverKeyId, user_id: Annotated[int, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], + solver_service: Annotated[SolverService, Depends(SolverService)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ) -> Solver: @@ -144,7 +144,7 @@ async def get_solver( # IMPORTANT: by adding /latest, we avoid changing the order of this entry in the router list # otherwise, {solver_key:path} will override and consume any of the paths that follow. try: - solver = await catalog_client.get_latest_release( + solver = await solver_service.get_latest_release( user_id=user_id, solver_key=solver_key, product_name=product_name, diff --git a/services/api-server/src/simcore_service_api_server/services_http/catalog.py b/services/api-server/src/simcore_service_api_server/services_http/catalog.py index 892066d8d769..a192dfa89fce 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_http/catalog.py @@ -4,7 +4,6 @@ from collections.abc import Callable from dataclasses import dataclass from functools import partial -from operator import attrgetter from typing import Final, Literal from fastapi import FastAPI, status @@ -145,32 +144,6 @@ async def list_services( services = [service for service in services if predicate(service)] return services - @_exception_mapper( - http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} - ) - async def _get_service( - self, *, user_id: int, name: SolverKeyId, version: VersionStr, product_name: str - ) -> TruncatedCatalogServiceOut: - - assert version != LATEST_VERSION # nosec - - service_key = urllib.parse.quote_plus(name) - service_version = version - - response = await self.client.get( - f"/services/{service_key}/{service_version}", - params={"user_id": user_id}, - headers={"x-simcore-products-name": product_name}, - ) - response.raise_for_status() - - service: ( - TruncatedCatalogServiceOut - ) = await asyncio.get_event_loop().run_in_executor( - None, _parse_response, TruncatedCatalogServiceOutAdapter, response - ) - return service - @_exception_mapper( http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} ) @@ -236,22 +209,6 @@ def _this_solver(solver: Solver) -> bool: solvers = [service.to_solver() for service in services] return [solver for solver in solvers if _this_solver(solver)] - async def get_latest_release( - self, - *, - user_id: int, - solver_key: SolverKeyId, - product_name: str, - ) -> Solver: - releases = await self.list_service_releases( - user_id=user_id, - solver_key=solver_key, - product_name=product_name, - ) - - # raises IndexError if None - return sorted(releases, key=attrgetter("pep404_version"))[-1] - # MODULES APP SETUP ------------------------------------------------------------- diff --git a/services/api-server/tests/unit/test_api_solvers.py b/services/api-server/tests/unit/test_api_solvers.py index d35b648629ea..e7ac79d0cc13 100644 --- a/services/api-server/tests/unit/test_api_solvers.py +++ b/services/api-server/tests/unit/test_api_solvers.py @@ -50,3 +50,26 @@ async def test_get_solver_pricing_plan( assert expected_status_code == response.status_code if response.status_code == status.HTTP_200_OK: _ = ServicePricingPlanGet.model_validate(response.json()) + + +@pytest.mark.parametrize( + "solver_key,expected_status_code", + [ + ("simcore/services/comp/valid_solver", status.HTTP_200_OK), + ], +) +async def test_get_latest_solver_release( + client: AsyncClient, + mocked_rpc_catalog_service_api, + auth: httpx.BasicAuth, + solver_key: str, + expected_status_code: int, +): + response = await client.get( + f"{API_VTAG}/solvers/{solver_key}/latest", + auth=auth, + ) + assert response.status_code == expected_status_code + if response.status_code == status.HTTP_200_OK: + assert "id" in response.json() + assert response.json()["id"] == solver_key From d07461e41b41ff08d15b250b8efc9ef7c18943ff Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 14:07:27 +0200 Subject: [PATCH 18/28] make pylint happy --- services/api-server/tests/unit/conftest.py | 5 ++++- services/api-server/tests/unit/test_api_programs.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/services/api-server/tests/unit/conftest.py b/services/api-server/tests/unit/conftest.py index c96fa8d8666a..bed207c212f9 100644 --- a/services/api-server/tests/unit/conftest.py +++ b/services/api-server/tests/unit/conftest.py @@ -65,7 +65,10 @@ def mocked_rpc_catalog_service_api( class MockRabbitMQRPCClient: pass - app.dependency_overrides[get_rabbitmq_rpc_client] = lambda: MockRabbitMQRPCClient() + def get_mock_rabbitmq_rpc_client(): + return MockRabbitMQRPCClient() + + app.dependency_overrides[get_rabbitmq_rpc_client] = get_mock_rabbitmq_rpc_client side_effects = CatalogRpcSideEffects() yield { diff --git a/services/api-server/tests/unit/test_api_programs.py b/services/api-server/tests/unit/test_api_programs.py index 22d369d39797..a0ad30eb1d12 100644 --- a/services/api-server/tests/unit/test_api_programs.py +++ b/services/api-server/tests/unit/test_api_programs.py @@ -1,3 +1,5 @@ +# pylint: disable=unused-argument +# pylint: disable=unused-variable import json from functools import partial from pathlib import Path From 52a97f281fdf331bd70db33cf0afe12577e2d020 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 14:25:58 +0200 Subject: [PATCH 19/28] fix webserver openapi test --- .../tests/unit/with_dbs/03/test__openapi_specs.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py b/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py index 9c6ad78f4a8d..886817bbf263 100644 --- a/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py +++ b/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py @@ -13,7 +13,6 @@ from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from pytest_simcore.openapi_specs import Entrypoint -from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.application import create_application from simcore_service_webserver.application_settings import get_application_settings from simcore_service_webserver.rest._utils import get_openapi_specs_path @@ -76,16 +75,7 @@ def test_app_named_resources_against_openapi_specs( openapi_specs_entrypoints: set[Entrypoint], app_rest_entrypoints: set[Entrypoint], ): - # remove task-legacy routes. These should not be exposed. - # this test compares directly against the openapi specs. In future it would be - # cleaner to compare against the fastapi app entry points in specs and - # avoid including the endpoints there - required_entry_points = { - e - for e in app_rest_entrypoints - if not e.path.startswith(f"/{API_VTAG}/tasks-legacy") - } - assert required_entry_points == openapi_specs_entrypoints + assert app_rest_entrypoints == openapi_specs_entrypoints # NOTE: missing here is: # - input schemas (path, query and body) From 105b4983386c13ff2f9a90bef083c030da11f262 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 15:04:43 +0200 Subject: [PATCH 20/28] Revert "fix webserver openapi test" This reverts commit 52a97f281fdf331bd70db33cf0afe12577e2d020. --- .../tests/unit/with_dbs/03/test__openapi_specs.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py b/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py index 886817bbf263..9c6ad78f4a8d 100644 --- a/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py +++ b/services/web/server/tests/unit/with_dbs/03/test__openapi_specs.py @@ -13,6 +13,7 @@ from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from pytest_simcore.openapi_specs import Entrypoint +from simcore_service_webserver._meta import API_VTAG from simcore_service_webserver.application import create_application from simcore_service_webserver.application_settings import get_application_settings from simcore_service_webserver.rest._utils import get_openapi_specs_path @@ -75,7 +76,16 @@ def test_app_named_resources_against_openapi_specs( openapi_specs_entrypoints: set[Entrypoint], app_rest_entrypoints: set[Entrypoint], ): - assert app_rest_entrypoints == openapi_specs_entrypoints + # remove task-legacy routes. These should not be exposed. + # this test compares directly against the openapi specs. In future it would be + # cleaner to compare against the fastapi app entry points in specs and + # avoid including the endpoints there + required_entry_points = { + e + for e in app_rest_entrypoints + if not e.path.startswith(f"/{API_VTAG}/tasks-legacy") + } + assert required_entry_points == openapi_specs_entrypoints # NOTE: missing here is: # - input schemas (path, query and body) From d67c526dc6ef70701576351270c39de5b69578d3 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 15:05:17 +0200 Subject: [PATCH 21/28] Revert "add legacy tasks to webserver" This reverts commit d94caa787e0422d4c58c141ff0be1b8ce5e4fd38. --- .../web-server/_long_running_tasks_legacy.py | 61 ---------------- api/specs/web-server/openapi.py | 1 - .../api/v0/openapi.yaml | 71 ------------------- 3 files changed, 133 deletions(-) delete mode 100644 api/specs/web-server/_long_running_tasks_legacy.py diff --git a/api/specs/web-server/_long_running_tasks_legacy.py b/api/specs/web-server/_long_running_tasks_legacy.py deleted file mode 100644 index fcbe3508c4dc..000000000000 --- a/api/specs/web-server/_long_running_tasks_legacy.py +++ /dev/null @@ -1,61 +0,0 @@ -# pylint: disable=redefined-outer-name -# pylint: disable=unused-argument -# pylint: disable=unused-variable -# pylint: disable=too-many-arguments - - -from typing import Annotated - -from fastapi import APIRouter, Depends, status -from models_library.generics import Envelope -from servicelib.aiohttp.long_running_tasks._routes import _PathParam -from servicelib.long_running_tasks._models import TaskGet, TaskStatus -from simcore_service_webserver._meta import API_VTAG - -router = APIRouter( - prefix=f"/{API_VTAG}/tasks-legacy", - tags=[ - "long-running-tasks-legacy", - ], -) - - -@router.get( - "", - response_model=Envelope[list[TaskGet]], - name="list_tasks", - description="Lists all long running tasks", -) -def list_tasks(): ... - - -@router.get( - "/{task_id}", - response_model=Envelope[TaskStatus], - name="get_task_status", - description="Retrieves the status of a task", -) -def get_task_status( - _path_params: Annotated[_PathParam, Depends()], -): ... - - -@router.delete( - "/{task_id}", - name="cancel_and_delete_task", - description="Cancels and deletes a task", - status_code=status.HTTP_204_NO_CONTENT, -) -def cancel_and_delete_task( - _path_params: Annotated[_PathParam, Depends()], -): ... - - -@router.get( - "/{task_id}/result", - name="get_task_result", - description="Retrieves the result of a task", -) -def get_task_result( - _path_params: Annotated[_PathParam, Depends()], -): ... diff --git a/api/specs/web-server/openapi.py b/api/specs/web-server/openapi.py index bb32e30f0520..b6fdf69a8e4e 100644 --- a/api/specs/web-server/openapi.py +++ b/api/specs/web-server/openapi.py @@ -36,7 +36,6 @@ "_exporter", "_folders", "_long_running_tasks", - "_long_running_tasks_legacy", "_licensed_items", "_licensed_items_purchases", "_licensed_items_checkouts", diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 273084e8e946..0e11c080bb71 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -3125,77 +3125,6 @@ paths: schema: $ref: '#/components/schemas/EnvelopedError' description: Internal Server Error - /v0/tasks-legacy: - get: - tags: - - long-running-tasks-legacy - summary: List Tasks - description: Lists all long running tasks - operationId: list_tasks - responses: - '200': - description: Successful Response - content: - application/json: - schema: - $ref: '#/components/schemas/Envelope_list_TaskGet__' - /v0/tasks-legacy/{task_id}: - get: - tags: - - long-running-tasks-legacy - summary: Get Task Status - description: Retrieves the status of a task - operationId: get_task_status - parameters: - - name: task_id - in: path - required: true - schema: - type: string - title: Task Id - responses: - '200': - description: Successful Response - content: - application/json: - schema: - $ref: '#/components/schemas/Envelope_TaskStatus_' - delete: - tags: - - long-running-tasks-legacy - summary: Cancel And Delete Task - description: Cancels and deletes a task - operationId: cancel_and_delete_task - parameters: - - name: task_id - in: path - required: true - schema: - type: string - title: Task Id - responses: - '204': - description: Successful Response - /v0/tasks-legacy/{task_id}/result: - get: - tags: - - long-running-tasks-legacy - summary: Get Task Result - description: Retrieves the result of a task - operationId: get_task_result - parameters: - - name: task_id - in: path - required: true - schema: - type: string - title: Task Id - responses: - '200': - description: Successful Response - content: - application/json: - schema: {} /v0/catalog/licensed-items: get: tags: From 35f80970bafa446ea5f63a953738eeb3aa50e8db Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 15:22:32 +0200 Subject: [PATCH 22/28] add exception handling in CatalogService --- .../exceptions/backend_errors.py | 14 +++++++++++-- .../services_http/catalog.py | 4 ++-- .../services_rpc/catalog.py | 21 +++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/exceptions/backend_errors.py b/services/api-server/src/simcore_service_api_server/exceptions/backend_errors.py index 3653038ed284..bde18b2dbb58 100644 --- a/services/api-server/src/simcore_service_api_server/exceptions/backend_errors.py +++ b/services/api-server/src/simcore_service_api_server/exceptions/backend_errors.py @@ -17,6 +17,11 @@ def named_fields(cls) -> set[str]: ) +class InvalidInputError(BaseBackEndError): + msg_template = "Invalid input" + status_code = status.HTTP_422_UNPROCESSABLE_ENTITY + + class ListSolversOrStudiesError(BaseBackEndError): msg_template = "Cannot list solvers/studies" status_code = status.HTTP_404_NOT_FOUND @@ -37,11 +42,16 @@ class ProfileNotFoundError(BaseBackEndError): status_code = status.HTTP_404_NOT_FOUND -class SolverOrStudyNotFoundError(BaseBackEndError): - msg_template = "Could not get solver/study {name}:{version}" +class ProgramOrSolverOrStudyNotFoundError(BaseBackEndError): + msg_template = "Could not get program/solver/study {name}:{version}" status_code = status.HTTP_404_NOT_FOUND +class ServiceForbiddenAccessError(BaseBackEndError): + msg_template = "Forbidden access to program/solver/study {name}:{version}" + status_code = status.HTTP_403_FORBIDDEN + + class JobNotFoundError(BaseBackEndError): msg_template = "Could not get solver/study job {project_id}" status_code = status.HTTP_404_NOT_FOUND diff --git a/services/api-server/src/simcore_service_api_server/services_http/catalog.py b/services/api-server/src/simcore_service_api_server/services_http/catalog.py index a192dfa89fce..0b2b46dc0619 100644 --- a/services/api-server/src/simcore_service_api_server/services_http/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_http/catalog.py @@ -17,7 +17,7 @@ from ..exceptions.backend_errors import ( ListSolversOrStudiesError, - SolverOrStudyNotFoundError, + ProgramOrSolverOrStudyNotFoundError, ) from ..exceptions.service_errors_utils import service_exception_mapper from ..models.basic_types import VersionStr @@ -145,7 +145,7 @@ async def list_services( return services @_exception_mapper( - http_status_map={status.HTTP_404_NOT_FOUND: SolverOrStudyNotFoundError} + http_status_map={status.HTTP_404_NOT_FOUND: ProgramOrSolverOrStudyNotFoundError} ) async def get_service_ports( self, diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index cf046fe76b63..bc31060928c0 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -1,3 +1,4 @@ +from functools import partial from typing import Annotated from fastapi import Depends @@ -12,10 +13,23 @@ from models_library.services_history import ServiceRelease from models_library.services_types import ServiceKey, ServiceVersion from models_library.users import UserID +from pydantic import ValidationError from servicelib.rabbitmq import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.catalog import services as catalog_rpc +from servicelib.rabbitmq.rpc_interfaces.catalog.errors import ( + CatalogForbiddenError, + CatalogItemNotFoundError, +) +from simcore_service_api_server.exceptions.backend_errors import ( + InvalidInputError, + ProgramOrSolverOrStudyNotFoundError, + ServiceForbiddenAccessError, +) from ..api.dependencies.rabbitmq import get_rabbitmq_rpc_client +from ..exceptions.service_errors_utils import service_exception_mapper + +_exception_mapper = partial(service_exception_mapper, service_name="CatalogService") class CatalogService: @@ -76,6 +90,13 @@ async def list_release_history( ) return page.data, meta + @_exception_mapper( + rpc_exception_map={ + CatalogItemNotFoundError: ProgramOrSolverOrStudyNotFoundError, + CatalogForbiddenError: ServiceForbiddenAccessError, + ValidationError: InvalidInputError, + } + ) async def get( self, *, From b69c304b311a2434e8679cca16c09f4714ceed0f Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 15:31:18 +0200 Subject: [PATCH 23/28] add error handling to CatalogService --- .../src/simcore_service_api_server/_service_programs.py | 4 ++-- .../src/simcore_service_api_server/_service_solvers.py | 8 ++++---- .../simcore_service_api_server/services_rpc/catalog.py | 8 ++++---- services/api-server/tests/unit/test_services_catalog.py | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py index f822c54dd6ff..a7f7ea4fe7cc 100644 --- a/services/api-server/src/simcore_service_api_server/_service_programs.py +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -26,8 +26,8 @@ async def get_program( ) -> Program: service = await self._catalog_service.get( user_id=user_id, - service_key=name, - service_version=version, + name=name, + version=version, product_name=product_name, ) assert service.service_type == ServiceType.DYNAMIC # nosec diff --git a/services/api-server/src/simcore_service_api_server/_service_solvers.py b/services/api-server/src/simcore_service_api_server/_service_solvers.py index da85552a5d77..961cf5425a88 100644 --- a/services/api-server/src/simcore_service_api_server/_service_solvers.py +++ b/services/api-server/src/simcore_service_api_server/_service_solvers.py @@ -34,8 +34,8 @@ async def get_solver( ) -> Solver: service = await self._catalog_service.get( user_id=user_id, - service_key=name, - service_version=version, + name=name, + version=version, product_name=product_name, ) assert ( # nosec @@ -66,8 +66,8 @@ async def get_latest_release( release = sorted(service_releases, key=lambda s: Version(s.version))[-1] service = await self._catalog_service.get( user_id=user_id, - service_key=solver_key, - service_version=release.version, + name=solver_key, + version=release.version, product_name=product_name, ) diff --git a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py index bc31060928c0..8ecf61bf88f0 100644 --- a/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py +++ b/services/api-server/src/simcore_service_api_server/services_rpc/catalog.py @@ -102,14 +102,14 @@ async def get( *, product_name: ProductName, user_id: UserID, - service_key: ServiceKey, - service_version: ServiceVersion, + name: ServiceKey, + version: ServiceVersion, ) -> ServiceGetV2: return await catalog_rpc.get_service( self._client, product_name=product_name, user_id=user_id, - service_key=service_key, - service_version=service_version, + service_key=name, + service_version=version, ) diff --git a/services/api-server/tests/unit/test_services_catalog.py b/services/api-server/tests/unit/test_services_catalog.py index 40d945023c18..767ec8698899 100644 --- a/services/api-server/tests/unit/test_services_catalog.py +++ b/services/api-server/tests/unit/test_services_catalog.py @@ -69,8 +69,8 @@ async def test_catalog_service_read_solvers( service: ServiceGetV2 = await catalog_service.get( product_name=product_name, user_id=user_id, - service_key=selected_solver.id, - service_version=oldest_release.version, + name=selected_solver.id, + version=oldest_release.version, ) solver = to_solver(service) assert solver.id == selected_solver.id From 27ec715ec85286c562b4681ae4ba1b8e3a829226 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Wed, 16 Apr 2025 15:49:26 +0200 Subject: [PATCH 24/28] @pcrespov clean up JobService --- .../_service_job.py | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/_service_job.py b/services/api-server/src/simcore_service_api_server/_service_job.py index d44827370076..1b7cd79a3171 100644 --- a/services/api-server/src/simcore_service_api_server/_service_job.py +++ b/services/api-server/src/simcore_service_api_server/_service_job.py @@ -8,6 +8,7 @@ from models_library.projects_nodes_io import NodeID from pydantic import HttpUrl from servicelib.fastapi.app_state import SingletonInAppStateMixin +from servicelib.logging_utils import log_context from .api.dependencies.webserver_http import get_webserver_session from .models.schemas.jobs import Job, JobInputs @@ -24,12 +25,12 @@ class JobService(SingletonInAppStateMixin): app_state_name = "JobService" - _webserver_api: AuthSession + _web_rest_api: AuthSession def __init__( - self, webserver_api: Annotated[AuthSession, Depends(get_webserver_session)] + self, web_rest_api: Annotated[AuthSession, Depends(get_webserver_session)] ): - self._webserver_api = webserver_api + self._web_rest_api = web_rest_api async def create_job( self, @@ -39,23 +40,25 @@ async def create_job( parent_project_uuid: ProjectID | None, parent_node_id: NodeID | None, url_for: Callable[..., HttpUrl], - hidden: bool + hidden: bool, ) -> tuple[Job, ProjectGet]: # creates NEW job as prototype pre_job = Job.create_job_from_solver_or_program( solver_or_program_name=solver_or_program.name, inputs=inputs ) - _logger.debug("Creating Job '%s'", pre_job.name) + with log_context( + logger=_logger, level=logging.DEBUG, msg=f"Creating job {pre_job.name}" + ): + project_in: ProjectCreateNew = create_new_project_for_job( + solver_or_program, pre_job, inputs + ) + new_project: ProjectGet = await self._web_rest_api.create_project( + project_in, + is_hidden=hidden, + parent_project_uuid=parent_project_uuid, + parent_node_id=parent_node_id, + ) - project_in: ProjectCreateNew = create_new_project_for_job( - solver_or_program, pre_job, inputs - ) - new_project: ProjectGet = await self._webserver_api.create_project( - project_in, - is_hidden=hidden, - parent_project_uuid=parent_project_uuid, - parent_node_id=parent_node_id, - ) assert new_project # nosec assert new_project.uuid == pre_job.id # nosec From fc9245f1ee65fb583b491bcd78fcfc0014fec641 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Thu, 17 Apr 2025 07:18:30 +0200 Subject: [PATCH 25/28] add Deprecated note @pcrespov --- services/api-server/openapi.json | 2 +- .../_service_programs.py | 4 +- .../_service_solvers.py | 4 +- .../api/routes/_constants.py | 38 +++++++++++++++++++ .../api/routes/programs.py | 6 +-- .../api/routes/solvers_jobs.py | 4 +- .../api/routes/solvers_jobs_getters.py | 18 ++++++++- 7 files changed, 63 insertions(+), 13 deletions(-) diff --git a/services/api-server/openapi.json b/services/api-server/openapi.json index 78309bd37ebd..5240853dab55 100644 --- a/services/api-server/openapi.json +++ b/services/api-server/openapi.json @@ -2356,7 +2356,7 @@ "solvers" ], "summary": "List Jobs", - "description": "List of jobs in a specific released solver (limited to 20 jobs)\n\n- DEPRECATION: This implementation and returned values are deprecated and the will be replaced by that of get_jobs_page\n- SEE `get_jobs_page` for paginated version of this function", + "description": "\ud83d\udea8 **Deprecated**: This endpoint is deprecated and will be removed in a future release.\nPlease use `GET /{solver_key}/releases/{version}/jobs/page` instead.\n\n\n\nList of jobs in a specific released solver\n\nNew in *version 0.5*\n\nRemoved in *version 0.7*: This endpoint is deprecated and will be removed in a future version", "operationId": "list_jobs", "security": [ { diff --git a/services/api-server/src/simcore_service_api_server/_service_programs.py b/services/api-server/src/simcore_service_api_server/_service_programs.py index a7f7ea4fe7cc..9f55d1379a86 100644 --- a/services/api-server/src/simcore_service_api_server/_service_programs.py +++ b/services/api-server/src/simcore_service_api_server/_service_programs.py @@ -11,9 +11,7 @@ class ProgramService: _catalog_service: CatalogService - def __init__( - self, _catalog_service: Annotated[CatalogService, Depends(CatalogService)] - ): + def __init__(self, _catalog_service: Annotated[CatalogService, Depends()]): self._catalog_service = _catalog_service async def get_program( diff --git a/services/api-server/src/simcore_service_api_server/_service_solvers.py b/services/api-server/src/simcore_service_api_server/_service_solvers.py index 961cf5425a88..5aa760863967 100644 --- a/services/api-server/src/simcore_service_api_server/_service_solvers.py +++ b/services/api-server/src/simcore_service_api_server/_service_solvers.py @@ -19,9 +19,7 @@ class SolverService: _catalog_service: CatalogService - def __init__( - self, catalog_service: Annotated[CatalogService, Depends(CatalogService)] - ): + def __init__(self, catalog_service: Annotated[CatalogService, Depends()]): self._catalog_service = catalog_service async def get_solver( diff --git a/services/api-server/src/simcore_service_api_server/api/routes/_constants.py b/services/api-server/src/simcore_service_api_server/api/routes/_constants.py index 5962551d3411..5e9f4eb73b67 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/_constants.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/_constants.py @@ -20,3 +20,41 @@ # removed on inputs/outputs in routes FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT: Final[str] = "Removed in *version {}*: {}\n" + +FMSG_DEPRECATED_ROUTE_NOTICE: Final[str] = ( + "🚨 **Deprecated**: This endpoint is deprecated and will be removed in a future release.\n" + "Please use `{}` instead.\n\n" +) + + +def create_route_description( + *, + base: str = "", + deprecated: bool = False, + alternative: str | None = None, # alternative + changelog: list[str] | None = None +) -> str: + """ + Builds a consistent route description with optional deprecation and changelog information. + + Args: + base (str): Main route description. + deprecated (tuple): (retirement_date, alternative_route) if deprecated. + changelog (List[str]): List of formatted changelog strings. + + Returns: + str: Final description string. + """ + parts = [] + + if deprecated: + assert alternative, "If deprecated, alternative must be provided" # nosec + parts.append(FMSG_DEPRECATED_ROUTE_NOTICE.format(alternative)) + + if base: + parts.append(base) + + if changelog: + parts.append("\n".join(changelog)) + + return "\n\n".join(parts) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index 46c33482909a..95147717c62c 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -68,7 +68,7 @@ async def get_program_release( program_key: ProgramKeyId, version: VersionStr, user_id: Annotated[int, Depends(get_current_user_id)], - program_service: Annotated[ProgramService, Depends(ProgramService)], + program_service: Annotated[ProgramService, Depends()], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ) -> Program: @@ -107,8 +107,8 @@ async def create_program_job( program_key: ProgramKeyId, version: VersionStr, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], - program_service: Annotated[ProgramService, Depends(ProgramService)], - job_service: Annotated[JobService, Depends(JobService)], + program_service: Annotated[ProgramService, Depends()], + job_service: Annotated[JobService, Depends()], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], x_simcore_parent_project_uuid: Annotated[ProjectID | None, Header()] = None, diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py index 9e0250ef20e0..37c97d45e860 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs.py @@ -95,8 +95,8 @@ async def create_solver_job( version: VersionStr, inputs: JobInputs, user_id: Annotated[PositiveInt, Depends(get_current_user_id)], - solver_service: Annotated[SolverService, Depends(SolverService)], - job_service: Annotated[JobService, Depends(JobService)], + solver_service: Annotated[SolverService, Depends()], + job_service: Annotated[JobService, Depends()], wb_api_rpc: Annotated[WbApiRpcClient, Depends(get_wb_api_rpc_client)], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py index 127d5468c2ee..828a0973d41c 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py @@ -55,7 +55,11 @@ from ..dependencies.rabbitmq import get_log_check_timeout, get_log_distributor from ..dependencies.services import get_api_client from ..dependencies.webserver_http import AuthSession, get_webserver_session -from ._constants import FMSG_CHANGELOG_NEW_IN_VERSION +from ._constants import ( + FMSG_CHANGELOG_NEW_IN_VERSION, + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT, + create_route_description, +) from .solvers_jobs import ( JOBS_STATUS_CODES, METADATA_STATUS_CODES, @@ -118,6 +122,18 @@ "/{solver_key:path}/releases/{version}/jobs", response_model=list[Job], responses=JOBS_STATUS_CODES, + description=create_route_description( + base="List of jobs in a specific released solver", + deprecated=True, + alternative="GET /{solver_key}/releases/{version}/jobs/page", + changelog=[ + FMSG_CHANGELOG_NEW_IN_VERSION.format("0.5"), + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format( + "0.7", + "This endpoint is deprecated and will be removed in a future version", + ), + ], + ), ) async def list_jobs( solver_key: SolverKeyId, From 7318dc100bc1cf5e5d7cb8acabcc6d5b74b8c9e8 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Thu, 17 Apr 2025 07:25:57 +0200 Subject: [PATCH 26/28] add deprecation warnings to all listing endpoints --- .../api/routes/files.py | 20 +++++++++++++- .../api/routes/solvers.py | 26 ++++++++++++++----- .../api/routes/solvers_jobs_getters.py | 6 +---- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/files.py b/services/api-server/src/simcore_service_api_server/api/routes/files.py index 04e35bc00f9e..96c832a5c88f 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/files.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/files.py @@ -29,6 +29,11 @@ get_upload_links_from_s3, ) from simcore_sdk.node_ports_common.filemanager import upload_path as storage_upload_path +from simcore_service_api_server.api.routes._constants import ( + FMSG_CHANGELOG_ADDED_IN_VERSION, + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT, + create_route_description, +) from starlette.datastructures import URL from starlette.responses import RedirectResponse @@ -132,7 +137,20 @@ async def _create_domain_file( return file -@router.get("", response_model=list[OutputFile], responses=_FILE_STATUS_CODES) +@router.get( + "", + response_model=list[OutputFile], + responses=_FILE_STATUS_CODES, + description=create_route_description( + base="Lists all files stored in the system", + deprecated=True, + alternative="GET /v0/files/page", + changelog=[ + FMSG_CHANGELOG_ADDED_IN_VERSION.format("0.5"), + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format("0.7"), + ], + ), +) async def list_files( storage_client: Annotated[StorageApi, Depends(get_api_client(StorageApi))], user_id: Annotated[int, Depends(get_current_user_id)], diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py index d67f2cb0d868..dc87cae00582 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py @@ -20,7 +20,11 @@ from ..dependencies.services import get_api_client from ..dependencies.webserver_http import AuthSession, get_webserver_session from ._common import API_SERVER_DEV_FEATURES_ENABLED -from ._constants import FMSG_CHANGELOG_NEW_IN_VERSION +from ._constants import ( + FMSG_CHANGELOG_NEW_IN_VERSION, + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT, + create_route_description, +) _logger = logging.getLogger(__name__) @@ -44,17 +48,27 @@ # Would be nice to have /solvers/foo/releases/latest or solvers/foo/releases/3 , similar to docker tagging -@router.get("", response_model=list[Solver], responses=_SOLVER_STATUS_CODES) +@router.get( + "", + response_model=list[Solver], + responses=_SOLVER_STATUS_CODES, + description=create_route_description( + base="Lists all available solvers (latest version)", + deprecated=True, + alternative="GET /v0/solvers/page", + changelog=[ + FMSG_CHANGELOG_NEW_IN_VERSION.format("0.5.0"), + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format("0.7"), + ], + ), +) async def list_solvers( user_id: Annotated[int, Depends(get_current_user_id)], catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ): - """Lists all available solvers (latest version) - - SEE get_solvers_page for paginated version of this function - """ + """Lists all available solvers (latest version)""" solvers: list[Solver] = await catalog_client.list_latest_releases( user_id=user_id, product_name=product_name ) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py index 828a0973d41c..b86595f3d92f 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers_jobs_getters.py @@ -144,11 +144,7 @@ async def list_jobs( url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], product_name: Annotated[str, Depends(get_product_name)], ): - """List of jobs in a specific released solver (limited to 20 jobs) - - - DEPRECATION: This implementation and returned values are deprecated and the will be replaced by that of get_jobs_page - - SEE `get_jobs_page` for paginated version of this function - """ + """List of jobs in a specific released solver (limited to 20 jobs)""" solver = await solver_service.get_solver( user_id=user_id, From 9f591b1a506a64ea172cbbbf5582c36cbce9c3eb Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Thu, 17 Apr 2025 07:29:24 +0200 Subject: [PATCH 27/28] fixes to deprecation warnings --- services/api-server/openapi.json | 4 ++-- .../src/simcore_service_api_server/api/routes/files.py | 7 +++++-- .../src/simcore_service_api_server/api/routes/solvers.py | 7 +++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/services/api-server/openapi.json b/services/api-server/openapi.json index 5240853dab55..9c0d63ed8e9e 100644 --- a/services/api-server/openapi.json +++ b/services/api-server/openapi.json @@ -223,7 +223,7 @@ "files" ], "summary": "List Files", - "description": "Lists all files stored in the system\n\nSEE `get_files_page` for a paginated version of this function", + "description": "\ud83d\udea8 **Deprecated**: This endpoint is deprecated and will be removed in a future release.\nPlease use `GET /v0/files/page` instead.\n\n\n\nLists all files stored in the system\n\nAdded in *version 0.5*: \n\nRemoved in *version 0.7*: This endpoint is deprecated and will be removed in a future version", "operationId": "list_files", "responses": { "200": { @@ -1414,7 +1414,7 @@ "solvers" ], "summary": "List Solvers", - "description": "Lists all available solvers (latest version)\n\nSEE get_solvers_page for paginated version of this function", + "description": "\ud83d\udea8 **Deprecated**: This endpoint is deprecated and will be removed in a future release.\nPlease use `GET /v0/solvers/page` instead.\n\n\n\nLists all available solvers (latest version)\n\nNew in *version 0.5.0*\n\nRemoved in *version 0.7*: This endpoint is deprecated and will be removed in a future version", "operationId": "list_solvers", "responses": { "200": { diff --git a/services/api-server/src/simcore_service_api_server/api/routes/files.py b/services/api-server/src/simcore_service_api_server/api/routes/files.py index 96c832a5c88f..6a4073eb2e38 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/files.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/files.py @@ -146,8 +146,11 @@ async def _create_domain_file( deprecated=True, alternative="GET /v0/files/page", changelog=[ - FMSG_CHANGELOG_ADDED_IN_VERSION.format("0.5"), - FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format("0.7"), + FMSG_CHANGELOG_ADDED_IN_VERSION.format("0.5", ""), + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format( + "0.7", + "This endpoint is deprecated and will be removed in a future version", + ), ], ), ) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py index dc87cae00582..da286e491cd9 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/solvers.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/solvers.py @@ -57,8 +57,11 @@ deprecated=True, alternative="GET /v0/solvers/page", changelog=[ - FMSG_CHANGELOG_NEW_IN_VERSION.format("0.5.0"), - FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format("0.7"), + FMSG_CHANGELOG_NEW_IN_VERSION.format("0.5.0", ""), + FMSG_CHANGELOG_REMOVED_IN_VERSION_FORMAT.format( + "0.7", + "This endpoint is deprecated and will be removed in a future version", + ), ], ), ) From c6ee0c814e0ac30e4b3eb63dd37da2472fdb59a5 Mon Sep 17 00:00:00 2001 From: Mads Bisgaard Date: Thu, 17 Apr 2025 07:37:28 +0200 Subject: [PATCH 28/28] remove listing programs endpoint --- .../api/routes/programs.py | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/services/api-server/src/simcore_service_api_server/api/routes/programs.py b/services/api-server/src/simcore_service_api_server/api/routes/programs.py index 95147717c62c..be2f27fe2ef5 100644 --- a/services/api-server/src/simcore_service_api_server/api/routes/programs.py +++ b/services/api-server/src/simcore_service_api_server/api/routes/programs.py @@ -1,6 +1,5 @@ import logging from collections.abc import Callable -from operator import attrgetter from typing import Annotated from fastapi import APIRouter, Depends, Header, HTTPException, status @@ -23,43 +22,12 @@ from ...models.basic_types import VersionStr from ...models.schemas.jobs import Job, JobInputs from ...models.schemas.programs import Program, ProgramKeyId -from ...services_http.catalog import CatalogApi from ..dependencies.authentication import get_current_user_id, get_product_name -from ..dependencies.services import get_api_client _logger = logging.getLogger(__name__) router = APIRouter() -@router.get("", response_model=list[Program], include_in_schema=False) -async def list_programs( - user_id: Annotated[int, Depends(get_current_user_id)], - catalog_client: Annotated[CatalogApi, Depends(get_api_client(CatalogApi))], - url_for: Annotated[Callable, Depends(get_reverse_url_mapper)], - product_name: Annotated[str, Depends(get_product_name)], -): - """Lists all available solvers (latest version) - - DEPRECATION: This implementation and returned values are deprecated - - """ - services = await catalog_client.list_services( - user_id=user_id, - product_name=product_name, - predicate=None, - type_filter="DYNAMIC", - ) - - programs = [service.to_program() for service in services] - - for program in programs: - program.url = url_for( - "get_program_release", program_key=program.id, version=program.version - ) - - return sorted(programs, key=attrgetter("id")) - - @router.get( "/{program_key:path}/releases/{version}", response_model=Program,