From 2a76de5d770540e1e3762d0d7c45c8dd622ab270 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 14:23:45 +0200 Subject: [PATCH 01/29] tests concept further --- .../tests/fastapi/test_lifespan_utils.py | 224 +++++++++++++++++- 1 file changed, 221 insertions(+), 3 deletions(-) diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index b3619815db8..e7ac1b8e859 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -1,9 +1,21 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +import logging from collections.abc import AsyncIterator +from typing import Any -import asgi_lifespan import pytest +from asgi_lifespan import LifespanManager as ASGILifespanManager +from common_library.errors_classes import OsparcErrorMixin from fastapi import FastAPI -from fastapi_lifespan_manager import State +from fastapi_lifespan_manager import LifespanManager, State +from pytest_mock import MockerFixture +from pytest_simcore.helpers.logging_tools import log_context from servicelib.fastapi.lifespan_utils import combine_lifespans @@ -24,7 +36,7 @@ async def cache_lifespan(app: FastAPI) -> AsyncIterator[State]: capsys.readouterr() - async with asgi_lifespan.LifespanManager(app): + async with ASGILifespanManager(app): messages = capsys.readouterr().out assert "setup DB" in messages @@ -38,3 +50,209 @@ async def cache_lifespan(app: FastAPI) -> AsyncIterator[State]: assert "setup CACHE" not in messages assert "shutdown DB" in messages assert "shutdown CACHE" in messages + + +@pytest.fixture +def postgres_lifespan() -> LifespanManager: + lifespan_manager = LifespanManager() + + @lifespan_manager.add + async def _setup_postgres_sync_engine(_) -> AsyncIterator[State]: + with log_context(logging.INFO, "postgres_sync_engine"): + # pass state to children + yield {"postgres": {"engine": "Some Engine"}} + + @lifespan_manager.add + async def _setup_postgres_async_engine(_, state: State) -> AsyncIterator[State]: + with log_context(logging.INFO, "postgres_async_engine"): + # pass state to children + + state["postgres"].update(aengine="Some Async Engine") + yield state + + return lifespan_manager + + +@pytest.fixture +def rabbitmq_lifespan() -> LifespanManager: + lifespan_manager = LifespanManager() + + @lifespan_manager.add + async def _setup_rabbitmq(app: FastAPI) -> AsyncIterator[State]: + with log_context(logging.INFO, "rabbitmq"): + + with pytest.raises(AttributeError, match="rabbitmq_rpc_server"): + _ = app.state.rabbitmq_rpc_server + + # pass state to children + yield {"rabbitmq_rpc_server": "Some RabbitMQ RPC Server"} + + return lifespan_manager + + +async def test_app_lifespan_composition( + postgres_lifespan: LifespanManager, rabbitmq_lifespan: LifespanManager +): + # The app has its own database and rpc-server to initialize + # this is how you connect the lifespans pre-defined in servicelib + + @postgres_lifespan.add + async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: + + with log_context(logging.INFO, "app database"): + assert state["postgres"] == { + "engine": "Some Engine", + "aengine": "Some Async Engine", + } + + with pytest.raises(AttributeError, match="database_engine"): + _ = app.state.database_engine + + app.state.database_engine = state["postgres"]["engine"] + + yield {} + + # tear-down stage + assert app.state.database_engine + + @rabbitmq_lifespan.add + async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: + with log_context(logging.INFO, "app rpc-server"): + assert "rabbitmq_rpc_server" in state + + app.state.rpc_server = state["rabbitmq_rpc_server"] + + yield {} + + # Composes lifepans + app_lifespan = LifespanManager() + app_lifespan.include(postgres_lifespan) + app_lifespan.include(rabbitmq_lifespan) + + app = FastAPI(lifespan=app_lifespan) + async with ASGILifespanManager(app) as asgi_manager: + + # asgi_manage state + assert asgi_manager._state == { # noqa: SLF001 + "postgres": { + "engine": "Some Engine", + "aengine": "Some Async Engine", + }, + "rabbitmq_rpc_server": "Some RabbitMQ RPC Server", + } + + # app state + assert app.state.database_engine + assert app.state.rpc_server + + # Logs shows lifespan execution: + # -> postgres_sync_engine starting ... + # -> postgres_async_engine starting ... + # -> app database starting ... + # -> rabbitmq starting ... + # -> app rpc-server starting ... + # <- app rpc-server done () + # <- rabbitmq done () + # <- app database done (1ms) + # <- postgres_async_engine done (1ms) + # <- postgres_sync_engine done (1ms) + + +class LifespanError(OsparcErrorMixin, RuntimeError): ... + + +class LifespanOnStartupError(LifespanError): + msg_template = "Failed during startup of {module}" + + +class LifespanOnShutdownError(LifespanError): + msg_template = "Failed during shutdown of {module}" + + +@pytest.fixture +def failing_lifespan_manager(mocker: MockerFixture): + startup_step = mocker.MagicMock() + shutdown_step = mocker.MagicMock() + handle_error = mocker.MagicMock() + + def raise_error(): + msg = "failing module" + raise RuntimeError(msg) + + async def setup_failing_on_startup(app: FastAPI) -> AsyncIterator[State]: + _name = setup_failing_on_startup.__name__ + + with log_context(logging.INFO, _name): + try: + raise_error() + startup_step(_name) + except RuntimeError as exc: + handle_error(_name, exc) + raise LifespanOnStartupError(module=_name) from exc + yield {} + shutdown_step(_name) + + async def setup_failing_on_shutdown(app: FastAPI) -> AsyncIterator[State]: + _name = setup_failing_on_shutdown.__name__ + + with log_context(logging.INFO, _name): + startup_step(_name) + yield {} + try: + raise_error() + shutdown_step(_name) + except RuntimeError as exc: + handle_error(_name, exc) + raise LifespanOnShutdownError(module=_name) from exc + + return { + "startup_step": startup_step, + "shutdown_step": shutdown_step, + "handle_error": handle_error, + "setup_failing_on_startup": setup_failing_on_startup, + "setup_failing_on_shutdown": setup_failing_on_shutdown, + } + + +async def test_app_lifespan_with_error_on_startup( + failing_lifespan_manager: dict[str, Any], +): + app_lifespan = LifespanManager() + app_lifespan.add(failing_lifespan_manager["setup_failing_on_startup"]) + app = FastAPI(lifespan=app_lifespan) + + with pytest.raises(LifespanOnStartupError) as err_info: + async with ASGILifespanManager(app): + ... + + exception = err_info.value + assert failing_lifespan_manager["handle_error"].called + assert not failing_lifespan_manager["startup_step"].called + assert not failing_lifespan_manager["shutdown_step"].called + assert exception.error_context() == { + "module": "setup_failing_on_startup", + "message": "Failed during startup of setup_failing_on_startup", + "code": "RuntimeError.LifespanError.LifespanOnStartupError", + } + + +async def test_app_lifespan_with_error_on_shutdown( + failing_lifespan_manager: dict[str, Any], +): + app_lifespan = LifespanManager() + app_lifespan.add(failing_lifespan_manager["setup_failing_on_shutdown"]) + app = FastAPI(lifespan=app_lifespan) + + with pytest.raises(LifespanOnShutdownError) as err_info: + async with ASGILifespanManager(app): + ... + + exception = err_info.value + assert failing_lifespan_manager["handle_error"].called + assert failing_lifespan_manager["startup_step"].called + assert not failing_lifespan_manager["shutdown_step"].called + assert exception.error_context() == { + "module": "setup_failing_on_shutdown", + "message": "Failed during shutdown of setup_failing_on_shutdown", + "code": "RuntimeError.LifespanError.LifespanOnShutdownError", + } From 7e287e2a7f4c653c2be466eb25e05ea10c86bfd8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 14:27:46 +0200 Subject: [PATCH 02/29] =?UTF-8?q?=F0=9F=90=9B=20Fix:=20Adjust=20sign=20han?= =?UTF-8?q?dling=20in=20timedelta=20formatting=20and=20improve=20DynamicIn?= =?UTF-8?q?dentFormatter=20initialization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/pytest_simcore/helpers/logging_tools.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py b/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py index 2bb29562d75..bcb5b716d98 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py @@ -22,7 +22,7 @@ def _timedelta_as_minute_second_ms(delta: datetime.timedelta) -> str: if int(milliseconds * 1000) != 0: result += f"{int(milliseconds*1000)}ms" - sign = "-" if total_seconds < 0 else "" + sign = "-" if total_seconds < 0 else "<1ms" return f"{sign}{result.strip()}" @@ -32,10 +32,10 @@ class DynamicIndentFormatter(logging.Formatter): _cls_indent_level: int = 0 _instance_indent_level: int = 0 - def __init__(self, fmt=None, datefmt=None, style="%"): + def __init__(self, fmt: str | None = None, *args, **kwargs): dynamic_fmt = fmt or "%(asctime)s %(levelname)s %(message)s" assert "message" in dynamic_fmt - super().__init__(dynamic_fmt, datefmt, style) + super().__init__(dynamic_fmt, *args, **kwargs) def format(self, record) -> str: original_message = record.msg From 3ce6b3698eea7ba1af26ddbee94a4d6a6fa410be Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 14:31:58 +0200 Subject: [PATCH 03/29] helpers --- .../service-library/tests/fastapi/test_lifespan_utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index e7ac1b8e859..5e80fb63d0e 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -110,7 +110,7 @@ async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: app.state.database_engine = state["postgres"]["engine"] - yield {} + yield {} # no update # tear-down stage assert app.state.database_engine @@ -145,14 +145,17 @@ async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: assert app.state.database_engine assert app.state.rpc_server + # NOTE: these are different states! + assert app.state._state != asgi_manager._state # noqa: SLF001 + # Logs shows lifespan execution: # -> postgres_sync_engine starting ... # -> postgres_async_engine starting ... # -> app database starting ... # -> rabbitmq starting ... # -> app rpc-server starting ... - # <- app rpc-server done () - # <- rabbitmq done () + # <- app rpc-server done (<1ms) + # <- rabbitmq done (<1ms) # <- app database done (1ms) # <- postgres_async_engine done (1ms) # <- postgres_sync_engine done (1ms) From 8d4b3bf2cbd795779340706bbbf7e13c29975e72 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 15:31:00 +0200 Subject: [PATCH 04/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Rename=20async=20?= =?UTF-8?q?engine=20creation=20function=20and=20update=20references?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../servicelib/aiohttp/db_asyncpg_engine.py | 5 +- .../src/servicelib/db_asyncpg_utils.py | 12 ++- .../servicelib/fastapi/db_asyncpg_engine.py | 4 +- .../servicelib/fastapi/postgres_lifespan.py | 31 +++++++ .../tests/fastapi/test_postgres_lifespan.py | 91 +++++++++++++++++++ .../modules/db/_asyncpg.py | 6 +- 6 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 packages/service-library/src/servicelib/fastapi/postgres_lifespan.py create mode 100644 packages/service-library/tests/fastapi/test_postgres_lifespan.py diff --git a/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py b/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py index 1163a479c68..88b0338dadf 100644 --- a/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py @@ -4,7 +4,6 @@ SEE migration aiopg->asyncpg https://github.com/ITISFoundation/osparc-simcore/issues/4529 """ - import logging from typing import Final @@ -16,7 +15,7 @@ ) from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_pg_database_ready +from ..db_asyncpg_utils import create_async_engine_and_database_ready from ..logging_utils import log_context APP_DB_ASYNC_ENGINE_KEY: Final[str] = f"{__name__ }.AsyncEngine" @@ -56,7 +55,7 @@ async def connect_to_db(app: web.Application, settings: PostgresSettings) -> Non "Connecting app[APP_DB_ASYNC_ENGINE_KEY] to postgres with %s", f"{settings=}", ): - engine = await create_async_engine_and_pg_database_ready(settings) + engine = await create_async_engine_and_database_ready(settings) _set_async_engine_to_app_state(app, engine) _logger.info( diff --git a/packages/service-library/src/servicelib/db_asyncpg_utils.py b/packages/service-library/src/servicelib/db_asyncpg_utils.py index 84430916824..6a7550c6743 100644 --- a/packages/service-library/src/servicelib/db_asyncpg_utils.py +++ b/packages/service-library/src/servicelib/db_asyncpg_utils.py @@ -4,9 +4,6 @@ from models_library.healthchecks import IsNonResponsive, IsResponsive, LivenessResult from settings_library.postgres import PostgresSettings -from simcore_postgres_database.utils_aiosqlalchemy import ( # type: ignore[import-not-found] # this on is unclear - raise_if_migration_not_ready, -) from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine from tenacity import retry @@ -17,7 +14,7 @@ @retry(**PostgresRetryPolicyUponInitialization(_logger).kwargs) -async def create_async_engine_and_pg_database_ready( +async def create_async_engine_and_database_ready( settings: PostgresSettings, ) -> AsyncEngine: """ @@ -26,6 +23,10 @@ async def create_async_engine_and_pg_database_ready( - waits until db data is migrated (i.e. ready to use) - returns engine """ + from simcore_postgres_database.utils_aiosqlalchemy import ( # type: ignore[import-not-found] # this on is unclear + raise_if_migration_not_ready, + ) + server_settings = None if settings.POSTGRES_CLIENT_NAME: server_settings = { @@ -43,9 +44,10 @@ async def create_async_engine_and_pg_database_ready( try: await raise_if_migration_not_ready(engine) - except Exception: + except Exception as exc: # NOTE: engine must be closed because retry will create a new engine await engine.dispose() + exc.add_note("Failed during migration check. Created engine disposed.") raise return engine diff --git a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py index 920f68008ae..086befe66be 100644 --- a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py @@ -7,7 +7,7 @@ ) from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_pg_database_ready +from ..db_asyncpg_utils import create_async_engine_and_database_ready from ..logging_utils import log_context _logger = logging.getLogger(__name__) @@ -19,7 +19,7 @@ async def connect_to_db(app: FastAPI, settings: PostgresSettings) -> None: logging.DEBUG, f"Connecting and migraging {settings.dsn_with_async_sqlalchemy}", ): - engine = await create_async_engine_and_pg_database_ready(settings) + engine = await create_async_engine_and_database_ready(settings) app.state.engine = engine _logger.debug( diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py new file mode 100644 index 00000000000..e4334eef248 --- /dev/null +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -0,0 +1,31 @@ +import logging +from collections.abc import AsyncIterator + +from fastapi import FastAPI +from fastapi_lifespan_manager import LifespanManager, State +from servicelib.logging_utils import log_catch, log_context +from settings_library.postgres import PostgresSettings +from sqlalchemy.ext.asyncio import AsyncEngine + +from ..db_asyncpg_utils import create_async_engine_and_database_ready + +_logger = logging.getLogger(__name__) + + +postgres_lifespan = LifespanManager() + + +@postgres_lifespan.add +async def setup_postgres_database(app: FastAPI) -> AsyncIterator[State]: + with log_context(_logger, logging.INFO, f"{__name__}"): + + pg_settings: PostgresSettings = app.state.settings.CATALOG_POSTGRES + + async_engine: AsyncEngine = await create_async_engine_and_database_ready( + pg_settings + ) + + yield {"postgres.async_engine": async_engine} + + with log_catch(_logger, reraise=False): + await async_engine.dispose() diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py new file mode 100644 index 00000000000..19202131e10 --- /dev/null +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -0,0 +1,91 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +from typing import Annotated, Any + +import pytest +from asgi_lifespan import LifespanManager as ASGILifespanManager +from fastapi import FastAPI +from fastapi_lifespan_manager import LifespanManager, State +from pydantic import Field +from pytest_mock import MockerFixture, MockType +from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict +from pytest_simcore.helpers.typing_env import EnvVarsDict +from servicelib.fastapi.postgres_lifespan import postgres_lifespan +from settings_library.application import BaseApplicationSettings +from settings_library.postgres import PostgresSettings + + +@pytest.fixture +def mock_create_async_engine_and_database_ready(mocker: MockerFixture) -> MockType: + return mocker.patch( + "servicelib.fastapi.postgres_lifespan.create_async_engine_and_database_ready", + return_value=mocker.AsyncMock(), + ) + + +@pytest.fixture +def app_environment(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict: + return setenvs_from_dict( + monkeypatch, PostgresSettings.model_json_schema()["examples"][0] + ) + + +async def test_setup_postgres_database_in_an_app( + is_pdb_enabled: bool, + app_environment: EnvVarsDict, + mock_create_async_engine_and_database_ready: MockType, +): + assert app_environment + + @postgres_lifespan.add + async def my_db_setup(app: FastAPI, state: State): + app.state.my_db_engine = state["postgres.async_engine"] + + assert ( + app.state.my_db_engine + == mock_create_async_engine_and_database_ready.return_value + ) + + yield + + # compose lifespans + app_lifespan = LifespanManager() + app_lifespan.include(postgres_lifespan) + + # define app + app = FastAPI(lifespan=app_lifespan) + + # settings + class AppSettings(BaseApplicationSettings): + CATALOG_POSTGRES: Annotated[ + PostgresSettings, + Field(json_schema_extra={"auto_default_from_env": True}), + ] + + app.state.settings = AppSettings.create_from_envs() + + async with ASGILifespanManager( + app, + startup_timeout=None if is_pdb_enabled else 10, + shutdown_timeout=None if is_pdb_enabled else 10, + ) as asgi_manager: + # Verify that the async engine was created + mock_create_async_engine_and_database_ready.assert_called_once_with( + app.state.settings.CATALOG_POSTGRES + ) + + # Verify that the async engine is in the lifespan manager state + assert "postgres.async_engine" in asgi_manager._state # noqa: SLF001 + assert app.state.my_db_engine + assert ( + app.state.my_db_engine + == asgi_manager._state["postgres.async_engine"] # noqa: SLF001 + ) + + # Verify that the engine was disposed + async_engine: Any = mock_create_async_engine_and_database_ready.return_value + async_engine.dispose.assert_called_once() diff --git a/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py b/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py index 188117d9c93..83fe1cf9204 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py @@ -1,7 +1,9 @@ import logging from fastapi import FastAPI -from servicelib.db_asyncpg_utils import create_async_engine_and_pg_database_ready +from servicelib.db_asyncpg_utils import ( + create_async_engine_and_database_ready, +) from servicelib.logging_utils import log_context from settings_library.postgres import PostgresSettings from simcore_postgres_database.utils_aiosqlalchemy import get_pg_engine_stateinfo @@ -15,7 +17,7 @@ async def asyncpg_connect_to_db(app: FastAPI, settings: PostgresSettings) -> Non logging.DEBUG, f"Connecting and migraging {settings.dsn_with_async_sqlalchemy}", ): - engine = await create_async_engine_and_pg_database_ready(settings) + engine = await create_async_engine_and_database_ready(settings) app.state.asyncpg_engine = engine _logger.debug( From c491f77cfa1bebc1b5b8657b5679ef1622d3058a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 15:45:52 +0200 Subject: [PATCH 05/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Introduce=20Postg?= =?UTF-8?q?resLifespanStateKeys=20enum=20for=20improved=20state=20manageme?= =?UTF-8?q?nt=20in=20lifespan=20setup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../servicelib/fastapi/postgres_lifespan.py | 17 +++++-- .../tests/fastapi/test_postgres_lifespan.py | 47 ++++++++++++------- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index e4334eef248..eaa57f7db74 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -1,7 +1,7 @@ import logging from collections.abc import AsyncIterator +from enum import Enum # Added import -from fastapi import FastAPI from fastapi_lifespan_manager import LifespanManager, State from servicelib.logging_utils import log_catch, log_context from settings_library.postgres import PostgresSettings @@ -15,17 +15,26 @@ postgres_lifespan = LifespanManager() +class PostgresLifespanStateKeys(str, Enum): + POSTGRES_SETTINGS = "postgres_settings" + POSTGRES_ASYNC_ENGINE = "postgres.async_engine" + + @postgres_lifespan.add -async def setup_postgres_database(app: FastAPI) -> AsyncIterator[State]: +async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: + with log_context(_logger, logging.INFO, f"{__name__}"): - pg_settings: PostgresSettings = app.state.settings.CATALOG_POSTGRES + pg_settings: PostgresSettings = state[ + PostgresLifespanStateKeys.POSTGRES_SETTINGS + ] + assert isinstance(pg_settings, PostgresSettings) # nosec async_engine: AsyncEngine = await create_async_engine_and_database_ready( pg_settings ) - yield {"postgres.async_engine": async_engine} + yield {PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE: async_engine} with log_catch(_logger, reraise=False): await async_engine.dispose() diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py index 19202131e10..67dafabcf89 100644 --- a/packages/service-library/tests/fastapi/test_postgres_lifespan.py +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -4,6 +4,7 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable +from collections.abc import AsyncIterator from typing import Annotated, Any import pytest @@ -14,7 +15,10 @@ from pytest_mock import MockerFixture, MockType from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict -from servicelib.fastapi.postgres_lifespan import postgres_lifespan +from servicelib.fastapi.postgres_lifespan import ( + PostgresLifespanStateKeys, + postgres_lifespan, +) from settings_library.application import BaseApplicationSettings from settings_library.postgres import PostgresSettings @@ -41,33 +45,39 @@ async def test_setup_postgres_database_in_an_app( ): assert app_environment - @postgres_lifespan.add - async def my_db_setup(app: FastAPI, state: State): - app.state.my_db_engine = state["postgres.async_engine"] + class AppSettings(BaseApplicationSettings): + CATALOG_POSTGRES: Annotated[ + PostgresSettings, + Field(json_schema_extra={"auto_default_from_env": True}), + ] + + async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: + app.state.settings = AppSettings.create_from_envs() + + yield { + PostgresLifespanStateKeys.POSTGRES_SETTINGS: app.state.settings.CATALOG_POSTGRES + } + + async def my_db_setup(app: FastAPI, state: State) -> AsyncIterator[State]: + app.state.my_db_engine = state[PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE] assert ( app.state.my_db_engine == mock_create_async_engine_and_database_ready.return_value ) - yield + yield {} # compose lifespans app_lifespan = LifespanManager() + app_lifespan.add(my_app_settings) + + postgres_lifespan.add(my_db_setup) app_lifespan.include(postgres_lifespan) # define app app = FastAPI(lifespan=app_lifespan) - # settings - class AppSettings(BaseApplicationSettings): - CATALOG_POSTGRES: Annotated[ - PostgresSettings, - Field(json_schema_extra={"auto_default_from_env": True}), - ] - - app.state.settings = AppSettings.create_from_envs() - async with ASGILifespanManager( app, startup_timeout=None if is_pdb_enabled else 10, @@ -79,11 +89,16 @@ class AppSettings(BaseApplicationSettings): ) # Verify that the async engine is in the lifespan manager state - assert "postgres.async_engine" in asgi_manager._state # noqa: SLF001 + assert ( + PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE + in asgi_manager._state # noqa: SLF001 + ) assert app.state.my_db_engine assert ( app.state.my_db_engine - == asgi_manager._state["postgres.async_engine"] # noqa: SLF001 + == asgi_manager._state[ # noqa: SLF001 + PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE + ] ) # Verify that the engine was disposed From 7532170c2adfc709d36b6eb4556262cd012ea3e8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 15:52:36 +0200 Subject: [PATCH 06/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Rename=20async=20?= =?UTF-8?q?engine=20creation=20function=20for=20clarity=20and=20deprecate?= =?UTF-8?q?=20old=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/aiohttp/db_asyncpg_engine.py | 4 ++-- packages/service-library/src/servicelib/db_async_engine.py | 6 ++++++ packages/service-library/src/servicelib/db_asyncpg_utils.py | 2 +- .../src/servicelib/fastapi/db_asyncpg_engine.py | 4 ++-- .../src/servicelib/fastapi/postgres_lifespan.py | 6 +++--- .../src/simcore_service_director_v2/modules/db/_asyncpg.py | 4 ++-- 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py b/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py index 88b0338dadf..598dea5984d 100644 --- a/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py @@ -15,7 +15,7 @@ ) from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_database_ready +from ..db_asyncpg_utils import create_async_engine_and_wait_for_database_ready from ..logging_utils import log_context APP_DB_ASYNC_ENGINE_KEY: Final[str] = f"{__name__ }.AsyncEngine" @@ -55,7 +55,7 @@ async def connect_to_db(app: web.Application, settings: PostgresSettings) -> Non "Connecting app[APP_DB_ASYNC_ENGINE_KEY] to postgres with %s", f"{settings=}", ): - engine = await create_async_engine_and_database_ready(settings) + engine = await create_async_engine_and_wait_for_database_ready(settings) _set_async_engine_to_app_state(app, engine) _logger.info( diff --git a/packages/service-library/src/servicelib/db_async_engine.py b/packages/service-library/src/servicelib/db_async_engine.py index cff73e77047..df0e2366ded 100644 --- a/packages/service-library/src/servicelib/db_async_engine.py +++ b/packages/service-library/src/servicelib/db_async_engine.py @@ -1,4 +1,5 @@ import logging +import warnings from fastapi import FastAPI from settings_library.postgres import PostgresSettings @@ -17,6 +18,11 @@ @retry(**PostgresRetryPolicyUponInitialization(_logger).kwargs) async def connect_to_db(app: FastAPI, settings: PostgresSettings) -> None: + warnings.warn( + "The 'connect_to_db' function is deprecated and will be removed in a future release. " + "Please use 'postgres_lifespan' instead for managing the database connection lifecycle.", + DeprecationWarning, + ) with log_context( _logger, logging.DEBUG, f"connection to db {settings.dsn_with_async_sqlalchemy}" ): diff --git a/packages/service-library/src/servicelib/db_asyncpg_utils.py b/packages/service-library/src/servicelib/db_asyncpg_utils.py index 6a7550c6743..6de36328c32 100644 --- a/packages/service-library/src/servicelib/db_asyncpg_utils.py +++ b/packages/service-library/src/servicelib/db_asyncpg_utils.py @@ -14,7 +14,7 @@ @retry(**PostgresRetryPolicyUponInitialization(_logger).kwargs) -async def create_async_engine_and_database_ready( +async def create_async_engine_and_wait_for_database_ready( settings: PostgresSettings, ) -> AsyncEngine: """ diff --git a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py index 086befe66be..85f7724e929 100644 --- a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py @@ -7,7 +7,7 @@ ) from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_database_ready +from ..db_asyncpg_utils import create_async_engine_and_wait_for_database_ready from ..logging_utils import log_context _logger = logging.getLogger(__name__) @@ -19,7 +19,7 @@ async def connect_to_db(app: FastAPI, settings: PostgresSettings) -> None: logging.DEBUG, f"Connecting and migraging {settings.dsn_with_async_sqlalchemy}", ): - engine = await create_async_engine_and_database_ready(settings) + engine = await create_async_engine_and_wait_for_database_ready(settings) app.state.engine = engine _logger.debug( diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index eaa57f7db74..ebe3c011bb6 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -7,7 +7,7 @@ from settings_library.postgres import PostgresSettings from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_database_ready +from ..db_asyncpg_utils import create_async_engine_and_wait_for_database_ready _logger = logging.getLogger(__name__) @@ -30,8 +30,8 @@ async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: ] assert isinstance(pg_settings, PostgresSettings) # nosec - async_engine: AsyncEngine = await create_async_engine_and_database_ready( - pg_settings + async_engine: AsyncEngine = ( + await create_async_engine_and_wait_for_database_ready(pg_settings) ) yield {PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE: async_engine} diff --git a/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py b/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py index 83fe1cf9204..c53d19cda71 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py @@ -2,7 +2,7 @@ from fastapi import FastAPI from servicelib.db_asyncpg_utils import ( - create_async_engine_and_database_ready, + create_async_engine_and_wait_for_database_ready, ) from servicelib.logging_utils import log_context from settings_library.postgres import PostgresSettings @@ -17,7 +17,7 @@ async def asyncpg_connect_to_db(app: FastAPI, settings: PostgresSettings) -> Non logging.DEBUG, f"Connecting and migraging {settings.dsn_with_async_sqlalchemy}", ): - engine = await create_async_engine_and_database_ready(settings) + engine = await create_async_engine_and_wait_for_database_ready(settings) app.state.asyncpg_engine = engine _logger.debug( From 23af7121adb8a1e9dcdfbf38934178b0e735fc76 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 15:57:51 +0200 Subject: [PATCH 07/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Rename=20async=20?= =?UTF-8?q?engine=20creation=20function=20for=20consistency=20and=20update?= =?UTF-8?q?=20references?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/aiohttp/db_asyncpg_engine.py | 4 ++-- packages/service-library/src/servicelib/db_asyncpg_utils.py | 2 +- .../src/servicelib/fastapi/db_asyncpg_engine.py | 4 ++-- .../src/servicelib/fastapi/postgres_lifespan.py | 6 +++--- .../service-library/tests/fastapi/test_postgres_lifespan.py | 6 ++++-- .../src/simcore_service_director_v2/modules/db/_asyncpg.py | 4 ++-- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py b/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py index 598dea5984d..88b0338dadf 100644 --- a/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py @@ -15,7 +15,7 @@ ) from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_wait_for_database_ready +from ..db_asyncpg_utils import create_async_engine_and_database_ready from ..logging_utils import log_context APP_DB_ASYNC_ENGINE_KEY: Final[str] = f"{__name__ }.AsyncEngine" @@ -55,7 +55,7 @@ async def connect_to_db(app: web.Application, settings: PostgresSettings) -> Non "Connecting app[APP_DB_ASYNC_ENGINE_KEY] to postgres with %s", f"{settings=}", ): - engine = await create_async_engine_and_wait_for_database_ready(settings) + engine = await create_async_engine_and_database_ready(settings) _set_async_engine_to_app_state(app, engine) _logger.info( diff --git a/packages/service-library/src/servicelib/db_asyncpg_utils.py b/packages/service-library/src/servicelib/db_asyncpg_utils.py index 6de36328c32..6a7550c6743 100644 --- a/packages/service-library/src/servicelib/db_asyncpg_utils.py +++ b/packages/service-library/src/servicelib/db_asyncpg_utils.py @@ -14,7 +14,7 @@ @retry(**PostgresRetryPolicyUponInitialization(_logger).kwargs) -async def create_async_engine_and_wait_for_database_ready( +async def create_async_engine_and_database_ready( settings: PostgresSettings, ) -> AsyncEngine: """ diff --git a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py index 85f7724e929..086befe66be 100644 --- a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py @@ -7,7 +7,7 @@ ) from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_wait_for_database_ready +from ..db_asyncpg_utils import create_async_engine_and_database_ready from ..logging_utils import log_context _logger = logging.getLogger(__name__) @@ -19,7 +19,7 @@ async def connect_to_db(app: FastAPI, settings: PostgresSettings) -> None: logging.DEBUG, f"Connecting and migraging {settings.dsn_with_async_sqlalchemy}", ): - engine = await create_async_engine_and_wait_for_database_ready(settings) + engine = await create_async_engine_and_database_ready(settings) app.state.engine = engine _logger.debug( diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index ebe3c011bb6..eaa57f7db74 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -7,7 +7,7 @@ from settings_library.postgres import PostgresSettings from sqlalchemy.ext.asyncio import AsyncEngine -from ..db_asyncpg_utils import create_async_engine_and_wait_for_database_ready +from ..db_asyncpg_utils import create_async_engine_and_database_ready _logger = logging.getLogger(__name__) @@ -30,8 +30,8 @@ async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: ] assert isinstance(pg_settings, PostgresSettings) # nosec - async_engine: AsyncEngine = ( - await create_async_engine_and_wait_for_database_ready(pg_settings) + async_engine: AsyncEngine = await create_async_engine_and_database_ready( + pg_settings ) yield {PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE: async_engine} diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py index 67dafabcf89..b43eb4abd07 100644 --- a/packages/service-library/tests/fastapi/test_postgres_lifespan.py +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -8,6 +8,7 @@ from typing import Annotated, Any import pytest +import servicelib.fastapi.postgres_lifespan from asgi_lifespan import LifespanManager as ASGILifespanManager from fastapi import FastAPI from fastapi_lifespan_manager import LifespanManager, State @@ -25,8 +26,9 @@ @pytest.fixture def mock_create_async_engine_and_database_ready(mocker: MockerFixture) -> MockType: - return mocker.patch( - "servicelib.fastapi.postgres_lifespan.create_async_engine_and_database_ready", + return mocker.patch.object( + servicelib.fastapi.postgres_lifespan, + "create_async_engine_and_database_ready", return_value=mocker.AsyncMock(), ) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py b/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py index c53d19cda71..83fe1cf9204 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/db/_asyncpg.py @@ -2,7 +2,7 @@ from fastapi import FastAPI from servicelib.db_asyncpg_utils import ( - create_async_engine_and_wait_for_database_ready, + create_async_engine_and_database_ready, ) from servicelib.logging_utils import log_context from settings_library.postgres import PostgresSettings @@ -17,7 +17,7 @@ async def asyncpg_connect_to_db(app: FastAPI, settings: PostgresSettings) -> Non logging.DEBUG, f"Connecting and migraging {settings.dsn_with_async_sqlalchemy}", ): - engine = await create_async_engine_and_wait_for_database_ready(settings) + engine = await create_async_engine_and_database_ready(settings) app.state.asyncpg_engine = engine _logger.debug( From 8305b9b963d3d92b22efa33ff302fb4078a942e4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 16:00:35 +0200 Subject: [PATCH 08/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Deprecate=20conne?= =?UTF-8?q?ct=5Fto=5Fdb=20function=20and=20recommend=20postgres=5Flifespan?= =?UTF-8?q?=20for=20database=20connection=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../service-library/src/servicelib/db_async_engine.py | 1 + .../src/servicelib/fastapi/db_asyncpg_engine.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/packages/service-library/src/servicelib/db_async_engine.py b/packages/service-library/src/servicelib/db_async_engine.py index df0e2366ded..dd9166e46fa 100644 --- a/packages/service-library/src/servicelib/db_async_engine.py +++ b/packages/service-library/src/servicelib/db_async_engine.py @@ -22,6 +22,7 @@ async def connect_to_db(app: FastAPI, settings: PostgresSettings) -> None: "The 'connect_to_db' function is deprecated and will be removed in a future release. " "Please use 'postgres_lifespan' instead for managing the database connection lifecycle.", DeprecationWarning, + stacklevel=2, ) with log_context( _logger, logging.DEBUG, f"connection to db {settings.dsn_with_async_sqlalchemy}" diff --git a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py index 086befe66be..8f472dc9b51 100644 --- a/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py +++ b/packages/service-library/src/servicelib/fastapi/db_asyncpg_engine.py @@ -1,4 +1,5 @@ import logging +import warnings from fastapi import FastAPI from settings_library.postgres import PostgresSettings @@ -14,6 +15,13 @@ async def connect_to_db(app: FastAPI, settings: PostgresSettings) -> None: + warnings.warn( + "The 'connect_to_db' function is deprecated and will be removed in a future release. " + "Please use 'postgres_lifespan' instead for managing the database connection lifecycle.", + DeprecationWarning, + stacklevel=2, + ) + with log_context( _logger, logging.DEBUG, From c4fe8160ca1ac6d546b1037626a91f46370577c1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 16:23:33 +0200 Subject: [PATCH 09/29] =?UTF-8?q?=E2=9C=A8=20Refactor=20catalog:=20Impleme?= =?UTF-8?q?nt=20application=20lifespan=20management=20with=20Postgres=20in?= =?UTF-8?q?tegration=20and=20cleanup=20old=20connection=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/application.py | 58 ++++++++++++++++--- .../simcore_service_catalog/core/events.py | 15 +---- .../src/simcore_service_catalog/db/events.py | 10 +++- services/catalog/tests/unit/conftest.py | 13 +++-- 4 files changed, 67 insertions(+), 29 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 7bedab76a31..bba8cd3facd 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -1,10 +1,16 @@ import logging +from collections.abc import AsyncIterator from fastapi import FastAPI from fastapi.middleware.gzip import GZipMiddleware +from fastapi_lifespan_manager import LifespanManager, State from models_library.basic_types import BootModeEnum from servicelib.fastapi import timing_middleware from servicelib.fastapi.openapi import override_fastapi_openapi_method +from servicelib.fastapi.postgres_lifespan import ( + PostgresLifespanStateKeys, + postgres_lifespan, +) from servicelib.fastapi.profiler import initialize_profiler from servicelib.fastapi.prometheus_instrumentation import ( setup_prometheus_instrumentation, @@ -12,13 +18,25 @@ from servicelib.fastapi.tracing import initialize_tracing from starlette.middleware.base import BaseHTTPMiddleware -from .._meta import API_VERSION, API_VTAG, APP_NAME, PROJECT_NAME, SUMMARY +from .._meta import ( + API_VERSION, + API_VTAG, + APP_NAME, + PROJECT_NAME, + SUMMARY, +) from ..api.rest.routes import setup_rest_api_routes from ..api.rpc.routes import setup_rpc_api_routes +from ..db.events import setup_database from ..exceptions.handlers import setup_exception_handlers from ..services.function_services import setup_function_services from ..services.rabbitmq import setup_rabbitmq -from .events import create_on_shutdown, create_on_startup +from .events import ( + create_on_shutdown, + create_on_startup, + flush_finished_banner, + flush_started_banner, +) from .settings import ApplicationSettings _logger = logging.getLogger(__name__) @@ -34,7 +52,31 @@ ) -def create_app(settings: ApplicationSettings | None = None) -> FastAPI: +async def _main_setup(app: FastAPI) -> AsyncIterator[State]: + flush_started_banner() + + settings: ApplicationSettings = app.state.settings + + yield { + PostgresLifespanStateKeys.POSTGRES_SETTINGS: settings.CATALOG_POSTGRES, + } + + flush_finished_banner() + + +def _create_app_lifespan(): + # app lifespan + app_lifespan = LifespanManager() + app_lifespan.add(_main_setup) + + # - postgres lifespan + postgres_lifespan.add(setup_database) + app_lifespan.include(postgres_lifespan) + + return app_lifespan + + +def create_app() -> FastAPI: # keep mostly quiet noisy loggers quiet_level: int = max( min(logging.root.level + _LOG_LEVEL_STEP, logging.CRITICAL), logging.WARNING @@ -42,10 +84,7 @@ def create_app(settings: ApplicationSettings | None = None) -> FastAPI: for name in _NOISY_LOGGERS: logging.getLogger(name).setLevel(quiet_level) - if settings is None: - settings = ApplicationSettings.create_from_envs() - - assert settings # nosec + settings = ApplicationSettings.create_from_envs() _logger.debug(settings.model_dump_json(indent=2)) app = FastAPI( @@ -57,6 +96,7 @@ def create_app(settings: ApplicationSettings | None = None) -> FastAPI: openapi_url=f"/api/{API_VTAG}/openapi.json", docs_url="/dev/doc", redoc_url=None, # default disabled + lifespan=_create_app_lifespan(), ) override_fastapi_openapi_method(app) @@ -73,11 +113,11 @@ def create_app(settings: ApplicationSettings | None = None) -> FastAPI: setup_function_services(app) setup_rabbitmq(app) - if app.state.settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED: + if settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED: setup_prometheus_instrumentation(app) # MIDDLEWARES - if app.state.settings.CATALOG_PROFILING: + if settings.CATALOG_PROFILING: initialize_profiler(app) if settings.SC_BOOT_MODE != BootModeEnum.PRODUCTION: diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index f22adbba4ec..15e47b82ef1 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -3,11 +3,9 @@ from typing import TypeAlias from fastapi import FastAPI -from servicelib.fastapi.db_asyncpg_engine import close_db_connection, connect_to_db from servicelib.logging_utils import log_context from .._meta import APP_FINISHED_BANNER_MSG, APP_STARTED_BANNER_MSG -from ..db.events import setup_default_product from ..services.director import close_director, setup_director from .background_tasks import start_registry_sync_task, stop_registry_sync_task @@ -17,23 +15,17 @@ EventCallable: TypeAlias = Callable[[], Awaitable[None]] -def _flush_started_banner() -> None: +def flush_started_banner() -> None: # WARNING: this function is spied in the tests print(APP_STARTED_BANNER_MSG, flush=True) # noqa: T201 -def _flush_finished_banner() -> None: +def flush_finished_banner() -> None: print(APP_FINISHED_BANNER_MSG, flush=True) # noqa: T201 def create_on_startup(app: FastAPI) -> EventCallable: async def _() -> None: - _flush_started_banner() - - # setup connection to pg db - if app.state.settings.CATALOG_POSTGRES: - await connect_to_db(app, app.state.settings.CATALOG_POSTGRES) - await setup_default_product(app) if app.state.settings.CATALOG_DIRECTOR: # setup connection to director @@ -56,10 +48,7 @@ async def _() -> None: try: await stop_registry_sync_task(app) await close_director(app) - await close_db_connection(app) except Exception: # pylint: disable=broad-except _logger.exception("Unexpected error while closing application") - _flush_finished_banner() - return _ diff --git a/services/catalog/src/simcore_service_catalog/db/events.py b/services/catalog/src/simcore_service_catalog/db/events.py index 42de4c38620..82702fd0f78 100644 --- a/services/catalog/src/simcore_service_catalog/db/events.py +++ b/services/catalog/src/simcore_service_catalog/db/events.py @@ -1,12 +1,20 @@ import logging +from collections.abc import AsyncIterator from fastapi import FastAPI +from fastapi_lifespan_manager import State +from servicelib.fastapi.postgres_lifespan import PostgresLifespanStateKeys from .repositories.products import ProductsRepository _logger = logging.getLogger(__name__) -async def setup_default_product(app: FastAPI): +async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: + app.state.engine = state[PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE] + repo = ProductsRepository(db_engine=app.state.engine) + app.state.default_product_name = await repo.get_default_product_name() + + yield {} diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 4b0000cbac4..4eac86a59d0 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -15,6 +15,7 @@ import pytest import respx import simcore_service_catalog +import simcore_service_catalog.core.application import simcore_service_catalog.core.events import yaml from asgi_lifespan import LifespanManager @@ -116,12 +117,12 @@ def spy_app(mocker: MockerFixture) -> AppLifeSpanSpyTargets: # work as expected return AppLifeSpanSpyTargets( on_startup=mocker.spy( - simcore_service_catalog.core.events, - "_flush_started_banner", + simcore_service_catalog.core.application, + "flush_started_banner", ), on_shutdown=mocker.spy( - simcore_service_catalog.core.events, - "_flush_finished_banner", + simcore_service_catalog.core.application, + "flush_finished_banner", ), ) @@ -139,7 +140,7 @@ async def app( # create instance assert app_environment - app_under_test = create_app(settings=app_settings) + app_under_test = create_app() assert spy_app.on_startup.call_count == 0 assert spy_app.on_shutdown.call_count == 0 @@ -166,7 +167,7 @@ def client( # create instance assert app_environment - app_under_test = create_app(settings=app_settings) + app_under_test = create_app() assert ( spy_app.on_startup.call_count == 0 From 5c0aa257ff441e1dbca3c333abfb02a2f932c924 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 17:28:51 +0200 Subject: [PATCH 10/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Ensure=20async=20?= =?UTF-8?q?engine=20disposal=20in=20setup=5Fpostgres=5Fdatabase=20with=20e?= =?UTF-8?q?rror=20handling=20and=20update=20test=20structure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../servicelib/fastapi/postgres_lifespan.py | 12 ++-- .../tests/fastapi/test_postgres_lifespan.py | 63 +++++++++++++++---- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index eaa57f7db74..9f730a71f5e 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -34,7 +34,11 @@ async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: pg_settings ) - yield {PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE: async_engine} - - with log_catch(_logger, reraise=False): - await async_engine.dispose() + try: + yield { + PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE: async_engine, + } + + finally: + with log_catch(_logger, reraise=False): + await async_engine.dispose() diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py index b43eb4abd07..8560a15da79 100644 --- a/packages/service-library/tests/fastapi/test_postgres_lifespan.py +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -40,11 +40,11 @@ def app_environment(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict: ) -async def test_setup_postgres_database_in_an_app( - is_pdb_enabled: bool, +@pytest.fixture +def app_lifespan( app_environment: EnvVarsDict, mock_create_async_engine_and_database_ready: MockType, -): +) -> LifespanManager: assert app_environment class AppSettings(BaseApplicationSettings): @@ -60,24 +60,28 @@ async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: PostgresLifespanStateKeys.POSTGRES_SETTINGS: app.state.settings.CATALOG_POSTGRES } - async def my_db_setup(app: FastAPI, state: State) -> AsyncIterator[State]: + async def my_database_setup(app: FastAPI, state: State) -> AsyncIterator[State]: app.state.my_db_engine = state[PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE] - assert ( - app.state.my_db_engine - == mock_create_async_engine_and_database_ready.return_value - ) - yield {} # compose lifespans app_lifespan = LifespanManager() app_lifespan.add(my_app_settings) - postgres_lifespan.add(my_db_setup) + postgres_lifespan.add(my_database_setup) app_lifespan.include(postgres_lifespan) - # define app + return app_lifespan + + +async def test_setup_postgres_database_in_an_app( + is_pdb_enabled: bool, + app_environment: EnvVarsDict, + mock_create_async_engine_and_database_ready: MockType, + app_lifespan: LifespanManager, +): + app = FastAPI(lifespan=app_lifespan) async with ASGILifespanManager( @@ -103,6 +107,43 @@ async def my_db_setup(app: FastAPI, state: State) -> AsyncIterator[State]: ] ) + assert ( + app.state.my_db_engine + == mock_create_async_engine_and_database_ready.return_value + ) + # Verify that the engine was disposed async_engine: Any = mock_create_async_engine_and_database_ready.return_value async_engine.dispose.assert_called_once() + + +async def test_setup_postgres_database_dispose_engine_on_failure( + is_pdb_enabled: bool, + app_environment: EnvVarsDict, + mock_create_async_engine_and_database_ready: MockType, + app_lifespan: LifespanManager, +): + expected_msg = "my_faulty_setup error" + + def raise_error(): + raise RuntimeError(expected_msg) + + @app_lifespan.add + async def my_faulty_setup(app: FastAPI, state: State) -> AsyncIterator[State]: + assert PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE in state + raise_error() + yield {} + + app = FastAPI(lifespan=app_lifespan) + + with pytest.raises(RuntimeError, match=expected_msg): + async with ASGILifespanManager( + app, + startup_timeout=None if is_pdb_enabled else 10, + shutdown_timeout=None if is_pdb_enabled else 10, + ): + ... + + # Verify that the engine was disposed even if error happend + async_engine: Any = mock_create_async_engine_and_database_ready.return_value + async_engine.dispose.assert_called_once() From 4dc9f461a5dfeb91078490f03662dfd3764e61a5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 18:09:19 +0200 Subject: [PATCH 11/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Clean=20up=20impo?= =?UTF-8?q?rts=20in=20postgres=5Flifespan.py=20for=20consistency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../pytest-simcore/src/pytest_simcore/helpers/logging_tools.py | 3 ++- .../src/servicelib/fastapi/postgres_lifespan.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py b/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py index bcb5b716d98..807d9f6b4b7 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/logging_tools.py @@ -32,7 +32,8 @@ class DynamicIndentFormatter(logging.Formatter): _cls_indent_level: int = 0 _instance_indent_level: int = 0 - def __init__(self, fmt: str | None = None, *args, **kwargs): + def __init__(self, *args, **kwargs): + fmt = args[0] if args else None dynamic_fmt = fmt or "%(asctime)s %(levelname)s %(message)s" assert "message" in dynamic_fmt super().__init__(dynamic_fmt, *args, **kwargs) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index 9f730a71f5e..dfe5a60aa36 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -1,6 +1,6 @@ import logging from collections.abc import AsyncIterator -from enum import Enum # Added import +from enum import Enum from fastapi_lifespan_manager import LifespanManager, State from servicelib.logging_utils import log_catch, log_context From f26338a25b6fd363bd136926d5b471c38c68529e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 18:27:36 +0200 Subject: [PATCH 12/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Enhance=20Postgre?= =?UTF-8?q?s=20settings=20validation=20and=20update=20test=20fixtures=20to?= =?UTF-8?q?=20use=20MockType?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/postgres_lifespan.py | 7 ++++++- services/catalog/tests/unit/conftest.py | 6 ++++-- services/catalog/tests/unit/test_api_rest.py | 5 +++-- services/catalog/tests/unit/test_utils_service_extras.py | 3 ++- services/catalog/tests/unit/test_utils_service_labels.py | 3 ++- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index dfe5a60aa36..dd94e49a4ec 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -25,9 +25,14 @@ async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: with log_context(_logger, logging.INFO, f"{__name__}"): - pg_settings: PostgresSettings = state[ + pg_settings: PostgresSettings | None = state[ PostgresLifespanStateKeys.POSTGRES_SETTINGS ] + + if pg_settings is None or not isinstance(pg_settings, PostgresSettings): + msg = f"Invalid {pg_settings=} on startup. Postgres cannot be disabled using settings" + raise RuntimeError(msg) + assert isinstance(pg_settings, PostgresSettings) # nosec async_engine: AsyncEngine = await create_async_engine_and_database_ready( diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 4eac86a59d0..f3059ea83ae 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -216,8 +216,10 @@ def service_caching_disabled(monkeypatch: pytest.MonkeyPatch) -> None: @pytest.fixture -def postgres_setup_disabled(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("CATALOG_POSTGRES", "null") +def postgres_setup_disabled(mocker: MockerFixture) -> MockType: + return mocker.patch.object( + simcore_service_catalog.core.application, "postgres_lifespan" + ) @pytest.fixture diff --git a/services/catalog/tests/unit/test_api_rest.py b/services/catalog/tests/unit/test_api_rest.py index 393c65abbc1..307b6c4c636 100644 --- a/services/catalog/tests/unit/test_api_rest.py +++ b/services/catalog/tests/unit/test_api_rest.py @@ -6,10 +6,11 @@ import httpx from fastapi import status from fastapi.testclient import TestClient +from pytest_mock import MockType def test_sync_client( - postgres_setup_disabled: None, + postgres_setup_disabled: MockType, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, director_setup_disabled: None, @@ -24,7 +25,7 @@ def test_sync_client( async def test_async_client( - postgres_setup_disabled: None, + postgres_setup_disabled: MockType, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, director_setup_disabled: None, diff --git a/services/catalog/tests/unit/test_utils_service_extras.py b/services/catalog/tests/unit/test_utils_service_extras.py index e5f69ab23bb..bd6d1f84ab5 100644 --- a/services/catalog/tests/unit/test_utils_service_extras.py +++ b/services/catalog/tests/unit/test_utils_service_extras.py @@ -8,6 +8,7 @@ from httpx import AsyncClient from models_library.api_schemas_directorv2.services import ServiceExtras from pydantic import TypeAdapter +from pytest_mock import MockType from respx import MockRouter @@ -17,7 +18,7 @@ def mock_engine(app: FastAPI) -> None: async def test_get_service_extras( - postgres_setup_disabled: None, + postgres_setup_disabled: MockType, mocked_director_service_api: MockRouter, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, diff --git a/services/catalog/tests/unit/test_utils_service_labels.py b/services/catalog/tests/unit/test_utils_service_labels.py index 14bbd06a286..485e01909f6 100644 --- a/services/catalog/tests/unit/test_utils_service_labels.py +++ b/services/catalog/tests/unit/test_utils_service_labels.py @@ -7,6 +7,7 @@ import pytest from fastapi import FastAPI, status from httpx import AsyncClient +from pytest_mock import MockType from respx import MockRouter @@ -16,7 +17,7 @@ def mock_engine(app: FastAPI) -> None: async def test_get_service_labels( - postgres_setup_disabled: None, + postgres_setup_disabled: MockType, mocked_director_service_api: MockRouter, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, From 119540c99181534ef7fb00b4822ce275afbd0d11 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 18:31:01 +0200 Subject: [PATCH 13/29] cleanup --- services/catalog/tests/unit/conftest.py | 11 +++++++---- services/catalog/tests/unit/test_services_director.py | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index f3059ea83ae..f4ac0466043 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -239,8 +239,9 @@ async def _side_effect(app: FastAPI): return _side_effect for name in ("start_registry_sync_task", "stop_registry_sync_task"): - mocker.patch( - f"simcore_service_catalog.core.events.{name}", + mocker.patch.object( + simcore_service_catalog.core.events, + name, side_effect=_factory(name), autospec=True, ) @@ -254,8 +255,10 @@ async def _side_effect(app: FastAPI): @pytest.fixture def rabbitmq_and_rpc_setup_disabled(mocker: MockerFixture): # The following services are affected if rabbitmq is not in place - mocker.patch("simcore_service_catalog.core.application.setup_rabbitmq") - mocker.patch("simcore_service_catalog.core.application.setup_rpc_api_routes") + mocker.patch.object(simcore_service_catalog.core.application, "setup_rabbitmq") + mocker.patch.object( + simcore_service_catalog.core.application, "setup_rpc_api_routes" + ) @pytest.fixture diff --git a/services/catalog/tests/unit/test_services_director.py b/services/catalog/tests/unit/test_services_director.py index eb36988b519..9078e02f476 100644 --- a/services/catalog/tests/unit/test_services_director.py +++ b/services/catalog/tests/unit/test_services_director.py @@ -12,6 +12,7 @@ import pytest from fastapi import FastAPI from models_library.services_metadata_published import ServiceMetaDataPublished +from pytest_mock import MockType from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from respx.router import MockRouter @@ -34,6 +35,7 @@ def app_environment( async def test_director_client_high_level_api( + postgres_setup_disabled: MockType, background_tasks_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, expected_director_list_services: list[dict[str, Any]], From 79887d24d56c4d09f1790a04fd742cf4cb01c925 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 18:49:22 +0200 Subject: [PATCH 14/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Simplify=20applic?= =?UTF-8?q?ation=20lifespan=20management=20by=20moving=20logic=20to=20even?= =?UTF-8?q?ts=20module=20and=20updating=20related=20handlers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/application.py | 44 ++-------------- .../simcore_service_catalog/core/events.py | 50 +++++++++++++++++-- services/catalog/tests/unit/conftest.py | 8 ++- .../tests/unit/test_services_director.py | 1 + 4 files changed, 55 insertions(+), 48 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index bba8cd3facd..11fd6ee655d 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -1,16 +1,10 @@ import logging -from collections.abc import AsyncIterator from fastapi import FastAPI from fastapi.middleware.gzip import GZipMiddleware -from fastapi_lifespan_manager import LifespanManager, State from models_library.basic_types import BootModeEnum from servicelib.fastapi import timing_middleware from servicelib.fastapi.openapi import override_fastapi_openapi_method -from servicelib.fastapi.postgres_lifespan import ( - PostgresLifespanStateKeys, - postgres_lifespan, -) from servicelib.fastapi.profiler import initialize_profiler from servicelib.fastapi.prometheus_instrumentation import ( setup_prometheus_instrumentation, @@ -27,15 +21,12 @@ ) from ..api.rest.routes import setup_rest_api_routes from ..api.rpc.routes import setup_rpc_api_routes -from ..db.events import setup_database from ..exceptions.handlers import setup_exception_handlers from ..services.function_services import setup_function_services from ..services.rabbitmq import setup_rabbitmq +from . import events from .events import ( - create_on_shutdown, - create_on_startup, - flush_finished_banner, - flush_started_banner, + _create_on_shutdown, ) from .settings import ApplicationSettings @@ -52,30 +43,6 @@ ) -async def _main_setup(app: FastAPI) -> AsyncIterator[State]: - flush_started_banner() - - settings: ApplicationSettings = app.state.settings - - yield { - PostgresLifespanStateKeys.POSTGRES_SETTINGS: settings.CATALOG_POSTGRES, - } - - flush_finished_banner() - - -def _create_app_lifespan(): - # app lifespan - app_lifespan = LifespanManager() - app_lifespan.add(_main_setup) - - # - postgres lifespan - postgres_lifespan.add(setup_database) - app_lifespan.include(postgres_lifespan) - - return app_lifespan - - def create_app() -> FastAPI: # keep mostly quiet noisy loggers quiet_level: int = max( @@ -96,7 +63,7 @@ def create_app() -> FastAPI: openapi_url=f"/api/{API_VTAG}/openapi.json", docs_url="/dev/doc", redoc_url=None, # default disabled - lifespan=_create_app_lifespan(), + lifespan=events.create_app_lifespan(), ) override_fastapi_openapi_method(app) @@ -106,9 +73,6 @@ def create_app() -> FastAPI: if settings.CATALOG_TRACING: initialize_tracing(app, settings.CATALOG_TRACING, APP_NAME) - # STARTUP-EVENT - app.add_event_handler("startup", create_on_startup(app)) - # PLUGIN SETUP setup_function_services(app) setup_rabbitmq(app) @@ -133,7 +97,7 @@ def create_app() -> FastAPI: setup_rpc_api_routes(app) # SHUTDOWN-EVENT - app.add_event_handler("shutdown", create_on_shutdown(app)) + app.add_event_handler("shutdown", _create_on_shutdown(app)) # EXCEPTIONS setup_exception_handlers(app) diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 15e47b82ef1..4959fe1bc18 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -1,13 +1,21 @@ +import contextlib import logging -from collections.abc import Awaitable, Callable +from collections.abc import AsyncIterator, Awaitable, Callable from typing import TypeAlias from fastapi import FastAPI +from fastapi_lifespan_manager import LifespanManager, State +from servicelib.fastapi.postgres_lifespan import ( + PostgresLifespanStateKeys, + postgres_lifespan, +) from servicelib.logging_utils import log_context from .._meta import APP_FINISHED_BANNER_MSG, APP_STARTED_BANNER_MSG +from ..db.events import setup_database from ..services.director import close_director, setup_director from .background_tasks import start_registry_sync_task, stop_registry_sync_task +from .settings import ApplicationSettings _logger = logging.getLogger(__name__) @@ -24,7 +32,19 @@ def flush_finished_banner() -> None: print(APP_FINISHED_BANNER_MSG, flush=True) # noqa: T201 -def create_on_startup(app: FastAPI) -> EventCallable: +async def _main_setup(app: FastAPI) -> AsyncIterator[State]: + flush_started_banner() + + settings: ApplicationSettings = app.state.settings + + yield { + PostgresLifespanStateKeys.POSTGRES_SETTINGS: settings.CATALOG_POSTGRES, + } + + flush_finished_banner() + + +def _create_on_startup(app: FastAPI) -> EventCallable: async def _() -> None: if app.state.settings.CATALOG_DIRECTOR: @@ -40,7 +60,7 @@ async def _() -> None: return _ -def create_on_shutdown(app: FastAPI) -> EventCallable: +def _create_on_shutdown(app: FastAPI) -> EventCallable: async def _() -> None: with log_context(_logger, logging.INFO, "Application shutdown"): @@ -52,3 +72,27 @@ async def _() -> None: _logger.exception("Unexpected error while closing application") return _ + + +@contextlib.asynccontextmanager +async def _other_setup(app: FastAPI) -> AsyncIterator[State]: + + await _create_on_startup(app)() + + yield {} + + await _create_on_shutdown(app)() + + +def create_app_lifespan(): + # app lifespan + app_lifespan = LifespanManager() + app_lifespan.add(_main_setup) + + # - postgres lifespan + postgres_lifespan.add(setup_database) + app_lifespan.include(postgres_lifespan) + + app_lifespan.add(_other_setup) + + return app_lifespan diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index f4ac0466043..c51cbe0d221 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -117,11 +117,11 @@ def spy_app(mocker: MockerFixture) -> AppLifeSpanSpyTargets: # work as expected return AppLifeSpanSpyTargets( on_startup=mocker.spy( - simcore_service_catalog.core.application, + simcore_service_catalog.core.events, "flush_started_banner", ), on_shutdown=mocker.spy( - simcore_service_catalog.core.application, + simcore_service_catalog.core.events, "flush_finished_banner", ), ) @@ -217,9 +217,7 @@ def service_caching_disabled(monkeypatch: pytest.MonkeyPatch) -> None: @pytest.fixture def postgres_setup_disabled(mocker: MockerFixture) -> MockType: - return mocker.patch.object( - simcore_service_catalog.core.application, "postgres_lifespan" - ) + return mocker.patch.object(simcore_service_catalog.core.events, "postgres_lifespan") @pytest.fixture diff --git a/services/catalog/tests/unit/test_services_director.py b/services/catalog/tests/unit/test_services_director.py index 9078e02f476..b422ee676c0 100644 --- a/services/catalog/tests/unit/test_services_director.py +++ b/services/catalog/tests/unit/test_services_director.py @@ -61,6 +61,7 @@ async def test_director_client_high_level_api( async def test_director_client_low_level_api( + postgres_setup_disabled: MockType, background_tasks_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, mocked_director_service_api: MockRouter, From 47979b4cf8a2482da637646d7c2c4d38da56bdc3 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 19:10:09 +0200 Subject: [PATCH 15/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Update=20service?= =?UTF-8?q?=20setup=20functions=20to=20use=20async=20iterators=20for=20imp?= =?UTF-8?q?roved=20lifecycle=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/application.py | 14 +--- .../core/background_tasks.py | 16 ++++- .../simcore_service_catalog/core/events.py | 70 +++++-------------- .../services/director.py | 25 +++---- .../services/function_services.py | 16 +++-- .../services/rabbitmq.py | 22 +++--- 6 files changed, 66 insertions(+), 97 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 11fd6ee655d..3e3f817a716 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -22,12 +22,7 @@ from ..api.rest.routes import setup_rest_api_routes from ..api.rpc.routes import setup_rpc_api_routes from ..exceptions.handlers import setup_exception_handlers -from ..services.function_services import setup_function_services -from ..services.rabbitmq import setup_rabbitmq from . import events -from .events import ( - _create_on_shutdown, -) from .settings import ApplicationSettings _logger = logging.getLogger(__name__) @@ -73,14 +68,10 @@ def create_app() -> FastAPI: if settings.CATALOG_TRACING: initialize_tracing(app, settings.CATALOG_TRACING, APP_NAME) - # PLUGIN SETUP - setup_function_services(app) - setup_rabbitmq(app) - + # MIDDLEWARES if settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED: setup_prometheus_instrumentation(app) - # MIDDLEWARES if settings.CATALOG_PROFILING: initialize_profiler(app) @@ -96,9 +87,6 @@ def create_app() -> FastAPI: setup_rest_api_routes(app, vtag=API_VTAG) setup_rpc_api_routes(app) - # SHUTDOWN-EVENT - app.add_event_handler("shutdown", _create_on_shutdown(app)) - # EXCEPTIONS setup_exception_handlers(app) diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py index cb269ee3919..0db5debbcb8 100644 --- a/services/catalog/src/simcore_service_catalog/core/background_tasks.py +++ b/services/catalog/src/simcore_service_catalog/core/background_tasks.py @@ -11,11 +11,13 @@ import asyncio import logging +from collections.abc import AsyncIterator from contextlib import suppress from pprint import pformat from typing import Final from fastapi import FastAPI, HTTPException +from fastapi_lifespan_manager import State from models_library.services import ServiceMetaDataPublished from models_library.services_types import ServiceKey, ServiceVersion from packaging.version import Version @@ -115,9 +117,9 @@ async def _ensure_registry_and_database_are_synced(app: FastAPI) -> None: director_api = get_director_api(app) services_in_manifest_map = await manifest.get_services_map(director_api) - services_in_db: set[ - tuple[ServiceKey, ServiceVersion] - ] = await _list_services_in_database(app.state.engine) + services_in_db: set[tuple[ServiceKey, ServiceVersion]] = ( + await _list_services_in_database(app.state.engine) + ) # check that the db has all the services at least once missing_services_in_db = set(services_in_manifest_map.keys()) - services_in_db @@ -232,3 +234,11 @@ async def stop_registry_sync_task(app: FastAPI) -> None: await task app.state.registry_sync_task = None _logger.info("registry syncing task stopped") + + +async def setup_background_task(app: FastAPI) -> AsyncIterator[State]: + await start_registry_sync_task(app) + try: + yield {} + finally: + await stop_registry_sync_task(app) diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 4959fe1bc18..dec57d8b7ff 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -1,7 +1,5 @@ -import contextlib import logging -from collections.abc import AsyncIterator, Awaitable, Callable -from typing import TypeAlias +from collections.abc import AsyncIterator from fastapi import FastAPI from fastapi_lifespan_manager import LifespanManager, State @@ -9,20 +7,18 @@ PostgresLifespanStateKeys, postgres_lifespan, ) -from servicelib.logging_utils import log_context from .._meta import APP_FINISHED_BANNER_MSG, APP_STARTED_BANNER_MSG from ..db.events import setup_database -from ..services.director import close_director, setup_director -from .background_tasks import start_registry_sync_task, stop_registry_sync_task +from ..services.director import setup_director +from ..services.function_services import setup_function_services +from ..services.rabbitmq import setup_rabbitmq +from .background_tasks import setup_background_task from .settings import ApplicationSettings _logger = logging.getLogger(__name__) -EventCallable: TypeAlias = Callable[[], Awaitable[None]] - - def flush_started_banner() -> None: # WARNING: this function is spied in the tests print(APP_STARTED_BANNER_MSG, flush=True) # noqa: T201 @@ -32,7 +28,7 @@ def flush_finished_banner() -> None: print(APP_FINISHED_BANNER_MSG, flush=True) # noqa: T201 -async def _main_setup(app: FastAPI) -> AsyncIterator[State]: +async def _setup_app(app: FastAPI) -> AsyncIterator[State]: flush_started_banner() settings: ApplicationSettings = app.state.settings @@ -44,55 +40,25 @@ async def _main_setup(app: FastAPI) -> AsyncIterator[State]: flush_finished_banner() -def _create_on_startup(app: FastAPI) -> EventCallable: - async def _() -> None: - - if app.state.settings.CATALOG_DIRECTOR: - # setup connection to director - await setup_director(app) - - # FIXME: check director service is in place and ready. Hand-shake?? - # SEE https://github.com/ITISFoundation/osparc-simcore/issues/1728 - await start_registry_sync_task(app) - - _logger.info("Application started") - - return _ - - -def _create_on_shutdown(app: FastAPI) -> EventCallable: - async def _() -> None: - - with log_context(_logger, logging.INFO, "Application shutdown"): - if app.state.settings.CATALOG_DIRECTOR: - try: - await stop_registry_sync_task(app) - await close_director(app) - except Exception: # pylint: disable=broad-except - _logger.exception("Unexpected error while closing application") - - return _ - - -@contextlib.asynccontextmanager -async def _other_setup(app: FastAPI) -> AsyncIterator[State]: - - await _create_on_startup(app)() - - yield {} - - await _create_on_shutdown(app)() - - def create_app_lifespan(): # app lifespan app_lifespan = LifespanManager() - app_lifespan.add(_main_setup) + app_lifespan.add(_setup_app) # - postgres lifespan postgres_lifespan.add(setup_database) app_lifespan.include(postgres_lifespan) - app_lifespan.add(_other_setup) + # - rabbitmq lifespan + app_lifespan.add(setup_rabbitmq) + + # - director lifespan + app_lifespan.add(setup_director) + + # - function services lifespan + app_lifespan.add(setup_function_services) + + # - background task lifespan + app_lifespan.add(setup_background_task) return app_lifespan diff --git a/services/catalog/src/simcore_service_catalog/services/director.py b/services/catalog/src/simcore_service_catalog/services/director.py index 52584062184..5cac7f037c3 100644 --- a/services/catalog/src/simcore_service_catalog/services/director.py +++ b/services/catalog/src/simcore_service_catalog/services/director.py @@ -3,7 +3,7 @@ import json import logging import urllib.parse -from collections.abc import Awaitable, Callable +from collections.abc import AsyncIterator, Awaitable, Callable from contextlib import suppress from pprint import pformat from typing import Any, Final @@ -11,6 +11,7 @@ import httpx from common_library.json_serialization import json_dumps from fastapi import FastAPI, HTTPException +from fastapi_lifespan_manager import State from models_library.api_schemas_directorv2.services import ServiceExtras from models_library.services_metadata_published import ServiceMetaDataPublished from models_library.services_types import ServiceKey, ServiceVersion @@ -65,7 +66,7 @@ def _validate_kind(entry_to_validate: dict[str, Any], kind_name: str): def _return_data_or_raise_error( - request_func: Callable[..., Awaitable[httpx.Response]] + request_func: Callable[..., Awaitable[httpx.Response]], ) -> Callable[..., Awaitable[list[Any] | dict[str, Any]]]: """ Creates a context for safe inter-process communication (IPC) @@ -288,13 +289,14 @@ async def get_service_extras( return TypeAdapter(ServiceExtras).validate_python(result) -async def setup_director(app: FastAPI) -> None: +async def setup_director(app: FastAPI) -> AsyncIterator[State]: + client: DirectorApi | None = None + if settings := app.state.settings.CATALOG_DIRECTOR: with log_context( _logger, logging.DEBUG, "Setup director at %s", f"{settings.base_url=}" ): async for attempt in AsyncRetrying(**_director_startup_retry_policy): - client = DirectorApi(base_url=settings.base_url, app=app) with attempt: client = DirectorApi(base_url=settings.base_url, app=app) if not await client.is_responsive(): @@ -303,17 +305,16 @@ async def setup_director(app: FastAPI) -> None: raise DirectorUnresponsiveError _logger.info( - "Connection to director-v0 succeded [%s]", + "Connection to director-v0 succeeded [%s]", json_dumps(attempt.retry_state.retry_object.statistics), ) # set when connected app.state.director_api = client - -async def close_director(app: FastAPI) -> None: - client: DirectorApi | None - if client := app.state.director_api: - await client.close() - - _logger.debug("Director client closed successfully") + try: + yield {} + finally: + if client: + await client.close() + _logger.debug("Director client closed successfully") diff --git a/services/catalog/src/simcore_service_catalog/services/function_services.py b/services/catalog/src/simcore_service_catalog/services/function_services.py index 7ed546f251b..9f939ec46fe 100644 --- a/services/catalog/src/simcore_service_catalog/services/function_services.py +++ b/services/catalog/src/simcore_service_catalog/services/function_services.py @@ -1,9 +1,12 @@ +from collections.abc import AsyncIterator + # mypy: disable-error-code=truthy-function from typing import Any from fastapi import status from fastapi.applications import FastAPI from fastapi.exceptions import HTTPException +from fastapi_lifespan_manager import State from models_library.function_services_catalog import ( is_function_service, iter_service_docker_data, @@ -31,12 +34,15 @@ def get_function_service(key, version) -> ServiceMetaDataPublished: ) from err -def setup_function_services(app: FastAPI): - def _on_startup() -> None: - catalog = [_as_dict(metadata) for metadata in iter_service_docker_data()] - app.state.frontend_services_catalog = catalog +async def setup_function_services(app: FastAPI) -> AsyncIterator[State]: + app.state.frontend_services_catalog = [ + _as_dict(metadata) for metadata in iter_service_docker_data() + ] - app.add_event_handler("startup", _on_startup) + try: + yield {} + finally: + app.state.frontend_services_catalog = None __all__: tuple[str, ...] = ( diff --git a/services/catalog/src/simcore_service_catalog/services/rabbitmq.py b/services/catalog/src/simcore_service_catalog/services/rabbitmq.py index 8400885efa0..c2b02259dd5 100644 --- a/services/catalog/src/simcore_service_catalog/services/rabbitmq.py +++ b/services/catalog/src/simcore_service_catalog/services/rabbitmq.py @@ -1,7 +1,9 @@ import logging +from collections.abc import AsyncIterator from typing import cast from fastapi import FastAPI +from fastapi_lifespan_manager import State from servicelib.rabbitmq import RabbitMQRPCClient, wait_till_rabbitmq_responsive from settings_library.rabbit import RabbitSettings @@ -15,25 +17,21 @@ def get_rabbitmq_settings(app: FastAPI) -> RabbitSettings: return settings -def setup_rabbitmq(app: FastAPI) -> None: +async def setup_rabbitmq(app: FastAPI) -> AsyncIterator[State]: settings: RabbitSettings = get_rabbitmq_settings(app) - app.state.rabbitmq_rpc_server = None + await wait_till_rabbitmq_responsive(settings.dsn) - async def _on_startup() -> None: - await wait_till_rabbitmq_responsive(settings.dsn) + app.state.rabbitmq_rpc_server = await RabbitMQRPCClient.create( + client_name=f"{PROJECT_NAME}_rpc_server", settings=settings + ) - app.state.rabbitmq_rpc_server = await RabbitMQRPCClient.create( - client_name=f"{PROJECT_NAME}_rpc_server", settings=settings - ) - - async def _on_shutdown() -> None: + try: + yield {} + finally: if app.state.rabbitmq_rpc_server: await app.state.rabbitmq_rpc_server.close() app.state.rabbitmq_rpc_server = None - app.add_event_handler("startup", _on_startup) - app.add_event_handler("shutdown", _on_shutdown) - def get_rabbitmq_rpc_server(app: FastAPI) -> RabbitMQRPCClient: assert app.state.rabbitmq_rpc_server # nosec From 05c1a90aee2ae757102f34e0b3976394b0ff2c6e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 19:24:19 +0200 Subject: [PATCH 16/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Integrate=20Prome?= =?UTF-8?q?theus=20instrumentation=20and=20tracing=20into=20application=20?= =?UTF-8?q?lifespan=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/application.py | 15 ----------- .../simcore_service_catalog/core/events.py | 26 ++++++++++++++++++- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 3e3f817a716..7be3a5b181e 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -5,17 +5,11 @@ from models_library.basic_types import BootModeEnum from servicelib.fastapi import timing_middleware from servicelib.fastapi.openapi import override_fastapi_openapi_method -from servicelib.fastapi.profiler import initialize_profiler -from servicelib.fastapi.prometheus_instrumentation import ( - setup_prometheus_instrumentation, -) -from servicelib.fastapi.tracing import initialize_tracing from starlette.middleware.base import BaseHTTPMiddleware from .._meta import ( API_VERSION, API_VTAG, - APP_NAME, PROJECT_NAME, SUMMARY, ) @@ -65,16 +59,7 @@ def create_app() -> FastAPI: # STATE app.state.settings = settings - if settings.CATALOG_TRACING: - initialize_tracing(app, settings.CATALOG_TRACING, APP_NAME) - # MIDDLEWARES - if settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED: - setup_prometheus_instrumentation(app) - - if settings.CATALOG_PROFILING: - initialize_profiler(app) - if settings.SC_BOOT_MODE != BootModeEnum.PRODUCTION: # middleware to time requests (ONLY for development) app.add_middleware( diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index dec57d8b7ff..4a002fa4787 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -7,8 +7,13 @@ PostgresLifespanStateKeys, postgres_lifespan, ) +from servicelib.fastapi.prometheus_instrumentation import ( + initialize_prometheus_instrumentation, + lifespan_prometheus_instrumentation, +) +from servicelib.fastapi.tracing import initialize_tracing -from .._meta import APP_FINISHED_BANNER_MSG, APP_STARTED_BANNER_MSG +from .._meta import APP_FINISHED_BANNER_MSG, APP_NAME, APP_STARTED_BANNER_MSG from ..db.events import setup_database from ..services.director import setup_director from ..services.function_services import setup_function_services @@ -33,18 +38,34 @@ async def _setup_app(app: FastAPI) -> AsyncIterator[State]: settings: ApplicationSettings = app.state.settings + if settings.CATALOG_TRACING: + initialize_tracing(app, settings.CATALOG_TRACING, APP_NAME) + yield { PostgresLifespanStateKeys.POSTGRES_SETTINGS: settings.CATALOG_POSTGRES, + "prometheus_instrumentation_enabled": settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED, } flush_finished_banner() +async def _setup_prometheus_instrumentation_adapter( + app: FastAPI, state: State +) -> AsyncIterator[State]: + enabled = state.get("prometheus_instrumentation_enabled", False) + if enabled: + initialize_prometheus_instrumentation(app) + async for _state in lifespan_prometheus_instrumentation(app): + yield _state + + def create_app_lifespan(): # app lifespan app_lifespan = LifespanManager() app_lifespan.add(_setup_app) + # WARNING: order matters + # - postgres lifespan postgres_lifespan.add(setup_database) app_lifespan.include(postgres_lifespan) @@ -61,4 +82,7 @@ def create_app_lifespan(): # - background task lifespan app_lifespan.add(setup_background_task) + # - prometheus instrumentation lifespan + app_lifespan.add(_setup_prometheus_instrumentation_adapter) + return app_lifespan From 4c26ada30ee19ad04ee6f0cdc49f3b4e59914a61 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 19:28:14 +0200 Subject: [PATCH 17/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Update=20RPC=20AP?= =?UTF-8?q?I=20route=20setup=20to=20use=20async=20iterator=20for=20improve?= =?UTF-8?q?d=20lifespan=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../simcore_service_catalog/api/rpc/routes.py | 16 ++++++++++------ .../simcore_service_catalog/core/application.py | 2 -- .../src/simcore_service_catalog/core/events.py | 4 ++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/routes.py b/services/catalog/src/simcore_service_catalog/api/rpc/routes.py index ce35e7867d1..f9960271d2c 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/routes.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/routes.py @@ -1,6 +1,8 @@ import logging +from collections.abc import AsyncIterator from fastapi import FastAPI +from fastapi_lifespan_manager import State from models_library.api_schemas_catalog import CATALOG_RPC_NAMESPACE from ...services.rabbitmq import get_rabbitmq_rpc_server @@ -9,9 +11,11 @@ _logger = logging.getLogger(__name__) -def setup_rpc_api_routes(app: FastAPI) -> None: - async def _on_startup() -> None: - rpc_server = get_rabbitmq_rpc_server(app) - await rpc_server.register_router(_services.router, CATALOG_RPC_NAMESPACE, app) - - app.add_event_handler("startup", _on_startup) +async def setup_rpc_api_routes(app: FastAPI) -> AsyncIterator[State]: + rpc_server = get_rabbitmq_rpc_server(app) + await rpc_server.register_router(_services.router, CATALOG_RPC_NAMESPACE, app) + try: + yield {} + finally: + # No specific cleanup required for now + pass diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 7be3a5b181e..2468e476e91 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -14,7 +14,6 @@ SUMMARY, ) from ..api.rest.routes import setup_rest_api_routes -from ..api.rpc.routes import setup_rpc_api_routes from ..exceptions.handlers import setup_exception_handlers from . import events from .settings import ApplicationSettings @@ -70,7 +69,6 @@ def create_app() -> FastAPI: # ROUTES setup_rest_api_routes(app, vtag=API_VTAG) - setup_rpc_api_routes(app) # EXCEPTIONS setup_exception_handlers(app) diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 4a002fa4787..05a771485d7 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -14,6 +14,7 @@ from servicelib.fastapi.tracing import initialize_tracing from .._meta import APP_FINISHED_BANNER_MSG, APP_NAME, APP_STARTED_BANNER_MSG +from ..api.rpc.routes import setup_rpc_api_routes from ..db.events import setup_database from ..services.director import setup_director from ..services.function_services import setup_function_services @@ -73,6 +74,9 @@ def create_app_lifespan(): # - rabbitmq lifespan app_lifespan.add(setup_rabbitmq) + # - rpc api routes lifespan + app_lifespan.add(setup_rpc_api_routes) + # - director lifespan app_lifespan.add(setup_director) From da317b7ed093c6c2ef81e754ef87b4c50fbf5e89 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 19:34:10 +0200 Subject: [PATCH 18/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Update=20backgrou?= =?UTF-8?q?nd=20tasks=20setup=20to=20use=20context=20manager=20for=20impro?= =?UTF-8?q?ved=20test=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/catalog/tests/unit/conftest.py | 37 ++++++++++++------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index c51cbe0d221..a8a6a49bd8a 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -224,25 +224,26 @@ def postgres_setup_disabled(mocker: MockerFixture) -> MockType: def background_tasks_setup_disabled(mocker: MockerFixture) -> None: """patch the setup of the background task so we can call it manually""" - def _factory(name): - async def _side_effect(app: FastAPI): - assert app + class MockedBackgroundTaskContextManager: + async def __aenter__(self): print( "TEST", background_tasks_setup_disabled.__name__, - "Disabled background tasks. Skipping execution of", - name, + "Disabled background tasks. Skipping execution of __aenter__", ) - return _side_effect + async def __aexit__(self, exc_type, exc_value, traceback): + print( + "TEST", + background_tasks_setup_disabled.__name__, + "Disabled background tasks. Skipping execution of __aexit__", + ) - for name in ("start_registry_sync_task", "stop_registry_sync_task"): - mocker.patch.object( - simcore_service_catalog.core.events, - name, - side_effect=_factory(name), - autospec=True, - ) + mocker.patch.object( + simcore_service_catalog.core.events, + "setup_background_task", + return_value=MockedBackgroundTaskContextManager(), + ) # @@ -253,10 +254,8 @@ async def _side_effect(app: FastAPI): @pytest.fixture def rabbitmq_and_rpc_setup_disabled(mocker: MockerFixture): # The following services are affected if rabbitmq is not in place - mocker.patch.object(simcore_service_catalog.core.application, "setup_rabbitmq") - mocker.patch.object( - simcore_service_catalog.core.application, "setup_rpc_api_routes" - ) + mocker.patch.object(simcore_service_catalog.core.events, "setup_rabbitmq") + mocker.patch.object(simcore_service_catalog.core.events, "setup_rpc_api_routes") @pytest.fixture @@ -272,8 +271,8 @@ async def rpc_client( @pytest.fixture -def director_setup_disabled(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("CATALOG_DIRECTOR", "null") +def director_setup_disabled(mocker: MockerFixture) -> None: + mocker.patch.object(simcore_service_catalog.core.events, "setup_director") @pytest.fixture From adcb226195a4315b178ae793c9e7e621be931c70 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 19:38:21 +0200 Subject: [PATCH 19/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Enhance=20Prometh?= =?UTF-8?q?eus=20instrumentation=20integration=20and=20update=20test=20fix?= =?UTF-8?q?tures=20for=20improved=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../catalog/src/simcore_service_catalog/core/application.py | 6 ++++++ services/catalog/src/simcore_service_catalog/core/events.py | 6 ++---- services/catalog/tests/unit/conftest.py | 4 ++-- services/catalog/tests/unit/test_api_rest.py | 5 ++--- services/catalog/tests/unit/test_services_director.py | 5 ++--- services/catalog/tests/unit/test_services_manifest.py | 1 + services/catalog/tests/unit/test_utils_service_extras.py | 3 +-- services/catalog/tests/unit/test_utils_service_labels.py | 3 +-- 8 files changed, 17 insertions(+), 16 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 2468e476e91..831030f4cfd 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -5,6 +5,9 @@ from models_library.basic_types import BootModeEnum from servicelib.fastapi import timing_middleware from servicelib.fastapi.openapi import override_fastapi_openapi_method +from servicelib.fastapi.prometheus_instrumentation import ( + initialize_prometheus_instrumentation, +) from starlette.middleware.base import BaseHTTPMiddleware from .._meta import ( @@ -59,6 +62,9 @@ def create_app() -> FastAPI: app.state.settings = settings # MIDDLEWARES + if settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED: + initialize_prometheus_instrumentation(app) + if settings.SC_BOOT_MODE != BootModeEnum.PRODUCTION: # middleware to time requests (ONLY for development) app.add_middleware( diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 05a771485d7..c348cd24388 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -8,7 +8,6 @@ postgres_lifespan, ) from servicelib.fastapi.prometheus_instrumentation import ( - initialize_prometheus_instrumentation, lifespan_prometheus_instrumentation, ) from servicelib.fastapi.tracing import initialize_tracing @@ -55,9 +54,8 @@ async def _setup_prometheus_instrumentation_adapter( ) -> AsyncIterator[State]: enabled = state.get("prometheus_instrumentation_enabled", False) if enabled: - initialize_prometheus_instrumentation(app) - async for _state in lifespan_prometheus_instrumentation(app): - yield _state + async for prometheus_state in lifespan_prometheus_instrumentation(app): + yield prometheus_state def create_app_lifespan(): diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index a8a6a49bd8a..7b11d57be10 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -216,8 +216,8 @@ def service_caching_disabled(monkeypatch: pytest.MonkeyPatch) -> None: @pytest.fixture -def postgres_setup_disabled(mocker: MockerFixture) -> MockType: - return mocker.patch.object(simcore_service_catalog.core.events, "postgres_lifespan") +def postgres_setup_disabled(mocker: MockerFixture): + mocker.patch.object(simcore_service_catalog.core.events, "postgres_lifespan") @pytest.fixture diff --git a/services/catalog/tests/unit/test_api_rest.py b/services/catalog/tests/unit/test_api_rest.py index 307b6c4c636..393c65abbc1 100644 --- a/services/catalog/tests/unit/test_api_rest.py +++ b/services/catalog/tests/unit/test_api_rest.py @@ -6,11 +6,10 @@ import httpx from fastapi import status from fastapi.testclient import TestClient -from pytest_mock import MockType def test_sync_client( - postgres_setup_disabled: MockType, + postgres_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, director_setup_disabled: None, @@ -25,7 +24,7 @@ def test_sync_client( async def test_async_client( - postgres_setup_disabled: MockType, + postgres_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, director_setup_disabled: None, diff --git a/services/catalog/tests/unit/test_services_director.py b/services/catalog/tests/unit/test_services_director.py index b422ee676c0..04a47cad08c 100644 --- a/services/catalog/tests/unit/test_services_director.py +++ b/services/catalog/tests/unit/test_services_director.py @@ -12,7 +12,6 @@ import pytest from fastapi import FastAPI from models_library.services_metadata_published import ServiceMetaDataPublished -from pytest_mock import MockType from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from respx.router import MockRouter @@ -35,7 +34,7 @@ def app_environment( async def test_director_client_high_level_api( - postgres_setup_disabled: MockType, + postgres_setup_disabled: None, background_tasks_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, expected_director_list_services: list[dict[str, Any]], @@ -61,7 +60,7 @@ async def test_director_client_high_level_api( async def test_director_client_low_level_api( - postgres_setup_disabled: MockType, + postgres_setup_disabled: None, background_tasks_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, mocked_director_service_api: MockRouter, diff --git a/services/catalog/tests/unit/test_services_manifest.py b/services/catalog/tests/unit/test_services_manifest.py index a43d82d5220..5dc354a21eb 100644 --- a/services/catalog/tests/unit/test_services_manifest.py +++ b/services/catalog/tests/unit/test_services_manifest.py @@ -33,6 +33,7 @@ def app_environment( async def test_services_manifest_api( + postgres_setup_disabled: None, rabbitmq_and_rpc_setup_disabled: None, mocked_director_service_api: MockRouter, app: FastAPI, diff --git a/services/catalog/tests/unit/test_utils_service_extras.py b/services/catalog/tests/unit/test_utils_service_extras.py index bd6d1f84ab5..e5f69ab23bb 100644 --- a/services/catalog/tests/unit/test_utils_service_extras.py +++ b/services/catalog/tests/unit/test_utils_service_extras.py @@ -8,7 +8,6 @@ from httpx import AsyncClient from models_library.api_schemas_directorv2.services import ServiceExtras from pydantic import TypeAdapter -from pytest_mock import MockType from respx import MockRouter @@ -18,7 +17,7 @@ def mock_engine(app: FastAPI) -> None: async def test_get_service_extras( - postgres_setup_disabled: MockType, + postgres_setup_disabled: None, mocked_director_service_api: MockRouter, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, diff --git a/services/catalog/tests/unit/test_utils_service_labels.py b/services/catalog/tests/unit/test_utils_service_labels.py index 485e01909f6..14bbd06a286 100644 --- a/services/catalog/tests/unit/test_utils_service_labels.py +++ b/services/catalog/tests/unit/test_utils_service_labels.py @@ -7,7 +7,6 @@ import pytest from fastapi import FastAPI, status from httpx import AsyncClient -from pytest_mock import MockType from respx import MockRouter @@ -17,7 +16,7 @@ def mock_engine(app: FastAPI) -> None: async def test_get_service_labels( - postgres_setup_disabled: MockType, + postgres_setup_disabled: None, mocked_director_service_api: MockRouter, rabbitmq_and_rpc_setup_disabled: None, background_tasks_setup_disabled: None, From 2bad4cd73a67bb3f8646df2fe02187b58818fb8e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 7 Apr 2025 19:56:41 +0200 Subject: [PATCH 20/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Rename=20postgres?= =?UTF-8?q?=20lifespan=20manager=20for=20consistency=20across=20the=20code?= =?UTF-8?q?base?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/postgres_lifespan.py | 4 ++-- .../service-library/tests/fastapi/test_lifespan_utils.py | 8 ++++---- .../tests/fastapi/test_postgres_lifespan.py | 6 +++--- .../catalog/src/simcore_service_catalog/core/events.py | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index dd94e49a4ec..a24f4c323a4 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -12,7 +12,7 @@ _logger = logging.getLogger(__name__) -postgres_lifespan = LifespanManager() +postgres_lifespan_manager = LifespanManager() class PostgresLifespanStateKeys(str, Enum): @@ -20,7 +20,7 @@ class PostgresLifespanStateKeys(str, Enum): POSTGRES_ASYNC_ENGINE = "postgres.async_engine" -@postgres_lifespan.add +@postgres_lifespan_manager.add async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: with log_context(_logger, logging.INFO, f"{__name__}"): diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index 5e80fb63d0e..97090c1d4a2 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -53,7 +53,7 @@ async def cache_lifespan(app: FastAPI) -> AsyncIterator[State]: @pytest.fixture -def postgres_lifespan() -> LifespanManager: +def postgres_lifespan_mng() -> LifespanManager: lifespan_manager = LifespanManager() @lifespan_manager.add @@ -91,12 +91,12 @@ async def _setup_rabbitmq(app: FastAPI) -> AsyncIterator[State]: async def test_app_lifespan_composition( - postgres_lifespan: LifespanManager, rabbitmq_lifespan: LifespanManager + postgres_lifespan_mng: LifespanManager, rabbitmq_lifespan: LifespanManager ): # The app has its own database and rpc-server to initialize # this is how you connect the lifespans pre-defined in servicelib - @postgres_lifespan.add + @postgres_lifespan_mng.add async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: with log_context(logging.INFO, "app database"): @@ -126,7 +126,7 @@ async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: # Composes lifepans app_lifespan = LifespanManager() - app_lifespan.include(postgres_lifespan) + app_lifespan.include(postgres_lifespan_mng) app_lifespan.include(rabbitmq_lifespan) app = FastAPI(lifespan=app_lifespan) diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py index 8560a15da79..4c96c79db1c 100644 --- a/packages/service-library/tests/fastapi/test_postgres_lifespan.py +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -18,7 +18,7 @@ from pytest_simcore.helpers.typing_env import EnvVarsDict from servicelib.fastapi.postgres_lifespan import ( PostgresLifespanStateKeys, - postgres_lifespan, + postgres_lifespan_manager, ) from settings_library.application import BaseApplicationSettings from settings_library.postgres import PostgresSettings @@ -69,8 +69,8 @@ async def my_database_setup(app: FastAPI, state: State) -> AsyncIterator[State]: app_lifespan = LifespanManager() app_lifespan.add(my_app_settings) - postgres_lifespan.add(my_database_setup) - app_lifespan.include(postgres_lifespan) + postgres_lifespan_manager.add(my_database_setup) + app_lifespan.include(postgres_lifespan_manager) return app_lifespan diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index c348cd24388..267ccdd20f7 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -5,7 +5,7 @@ from fastapi_lifespan_manager import LifespanManager, State from servicelib.fastapi.postgres_lifespan import ( PostgresLifespanStateKeys, - postgres_lifespan, + postgres_lifespan_manager, ) from servicelib.fastapi.prometheus_instrumentation import ( lifespan_prometheus_instrumentation, @@ -66,8 +66,8 @@ def create_app_lifespan(): # WARNING: order matters # - postgres lifespan - postgres_lifespan.add(setup_database) - app_lifespan.include(postgres_lifespan) + postgres_lifespan_manager.add(setup_database) + app_lifespan.include(postgres_lifespan_manager) # - rabbitmq lifespan app_lifespan.add(setup_rabbitmq) From f8bd194ffe32047ee941777fd47a5489353298a0 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 10:53:45 +0200 Subject: [PATCH 21/29] minor --- scripts/maintenance/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maintenance/requirements.txt b/scripts/maintenance/requirements.txt index 9b44c996137..4c521ce0a8c 100644 --- a/scripts/maintenance/requirements.txt +++ b/scripts/maintenance/requirements.txt @@ -11,4 +11,4 @@ pylint python-dateutil python-dotenv tenacity -typer[all] +typer From 85703c0999b9d29575bf54cdb2a87a066edd14fa Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 11:12:02 +0200 Subject: [PATCH 22/29] @sanderegg review: config error --- .../src/servicelib/fastapi/lifespan_utils.py | 15 ++++++++++-- .../servicelib/fastapi/postgres_lifespan.py | 8 +++++-- .../tests/fastapi/test_lifespan_utils.py | 18 ++++----------- .../tests/fastapi/test_postgres_lifespan.py | 23 +++++++++++++++++++ 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py index ee2808078e6..05d70104a17 100644 --- a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py +++ b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py @@ -1,13 +1,24 @@ from collections.abc import AsyncIterator from typing import Protocol +from common_library.errors_classes import OsparcErrorMixin from fastapi import FastAPI from fastapi_lifespan_manager import LifespanManager, State +class LifespanError(OsparcErrorMixin, RuntimeError): ... + + +class LifespanOnStartupError(LifespanError): + msg_template = "Failed during startup of {module}" + + +class LifespanOnShutdownError(LifespanError): + msg_template = "Failed during shutdown of {module}" + + class LifespanGenerator(Protocol): - def __call__(self, app: FastAPI) -> AsyncIterator["State"]: - ... + def __call__(self, app: FastAPI) -> AsyncIterator["State"]: ... def combine_lifespans(*generators: LifespanGenerator) -> LifespanManager: diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index a24f4c323a4..8c4c69d5b80 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -8,6 +8,7 @@ from sqlalchemy.ext.asyncio import AsyncEngine from ..db_asyncpg_utils import create_async_engine_and_database_ready +from .lifespan_utils import LifespanOnStartupError _logger = logging.getLogger(__name__) @@ -20,6 +21,10 @@ class PostgresLifespanStateKeys(str, Enum): POSTGRES_ASYNC_ENGINE = "postgres.async_engine" +class PostgresConfigurationError(LifespanOnStartupError): + msg_template = "Invalid postgres settings [={pg_settings}] on startup. Note that postgres cannot be disabled using settings" + + @postgres_lifespan_manager.add async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: @@ -30,8 +35,7 @@ async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: ] if pg_settings is None or not isinstance(pg_settings, PostgresSettings): - msg = f"Invalid {pg_settings=} on startup. Postgres cannot be disabled using settings" - raise RuntimeError(msg) + raise PostgresConfigurationError(pg_settings=pg_settings, module="postgres") assert isinstance(pg_settings, PostgresSettings) # nosec diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index 97090c1d4a2..e0dc1436ffb 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -11,12 +11,15 @@ import pytest from asgi_lifespan import LifespanManager as ASGILifespanManager -from common_library.errors_classes import OsparcErrorMixin from fastapi import FastAPI from fastapi_lifespan_manager import LifespanManager, State from pytest_mock import MockerFixture from pytest_simcore.helpers.logging_tools import log_context -from servicelib.fastapi.lifespan_utils import combine_lifespans +from servicelib.fastapi.lifespan_utils import ( + LifespanOnShutdownError, + LifespanOnStartupError, + combine_lifespans, +) async def test_multiple_lifespan_managers(capsys: pytest.CaptureFixture): @@ -161,17 +164,6 @@ async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: # <- postgres_sync_engine done (1ms) -class LifespanError(OsparcErrorMixin, RuntimeError): ... - - -class LifespanOnStartupError(LifespanError): - msg_template = "Failed during startup of {module}" - - -class LifespanOnShutdownError(LifespanError): - msg_template = "Failed during shutdown of {module}" - - @pytest.fixture def failing_lifespan_manager(mocker: MockerFixture): startup_step = mocker.MagicMock() diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py index 4c96c79db1c..0635daa0a2d 100644 --- a/packages/service-library/tests/fastapi/test_postgres_lifespan.py +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -17,6 +17,7 @@ from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict from servicelib.fastapi.postgres_lifespan import ( + PostgresConfigurationError, PostgresLifespanStateKeys, postgres_lifespan_manager, ) @@ -147,3 +148,25 @@ async def my_faulty_setup(app: FastAPI, state: State) -> AsyncIterator[State]: # Verify that the engine was disposed even if error happend async_engine: Any = mock_create_async_engine_and_database_ready.return_value async_engine.dispose.assert_called_once() + + +async def test_setup_postgres_database_with_empty_pg_settings( + is_pdb_enabled: bool, +): + async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: + yield {PostgresLifespanStateKeys.POSTGRES_SETTINGS: None} + + app_lifespan_manager = LifespanManager() + app_lifespan_manager.add(my_app_settings) + + app_lifespan_manager.include(postgres_lifespan_manager) + + app = FastAPI(lifespan=app_lifespan_manager) + + with pytest.raises(PostgresConfigurationError, match="postgres cannot be disabled"): + async with ASGILifespanManager( + app, + startup_timeout=None if is_pdb_enabled else 10, + shutdown_timeout=None if is_pdb_enabled else 10, + ): + ... From 2a33aa9395ad6f5fa4013090faf5e7b450d9d0da Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 14:40:26 +0200 Subject: [PATCH 23/29] @GitHK review: rename --- .../src/servicelib/db_asyncpg_utils.py | 2 +- .../servicelib/fastapi/postgres_lifespan.py | 35 ++++++++-------- .../tests/fastapi/test_lifespan_utils.py | 42 +++++++++---------- .../tests/fastapi/test_postgres_lifespan.py | 38 ++++++++--------- .../simcore_service_catalog/api/rpc/routes.py | 2 +- .../core/background_tasks.py | 2 +- .../simcore_service_catalog/core/events.py | 40 ++++++++---------- .../src/simcore_service_catalog/db/events.py | 6 +-- .../services/director.py | 2 +- .../services/function_services.py | 4 +- .../services/rabbitmq.py | 2 +- 11 files changed, 86 insertions(+), 89 deletions(-) diff --git a/packages/service-library/src/servicelib/db_asyncpg_utils.py b/packages/service-library/src/servicelib/db_asyncpg_utils.py index 6a7550c6743..4abbcd3ac66 100644 --- a/packages/service-library/src/servicelib/db_asyncpg_utils.py +++ b/packages/service-library/src/servicelib/db_asyncpg_utils.py @@ -33,7 +33,7 @@ async def create_async_engine_and_database_ready( "application_name": settings.POSTGRES_CLIENT_NAME, } - engine: AsyncEngine = create_async_engine( + engine = create_async_engine( settings.dsn_with_async_sqlalchemy, pool_size=settings.POSTGRES_MINSIZE, max_overflow=settings.POSTGRES_MAXSIZE - settings.POSTGRES_MINSIZE, diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index 8c4c69d5b80..def76edd62a 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -1,8 +1,9 @@ +import asyncio import logging from collections.abc import AsyncIterator from enum import Enum -from fastapi_lifespan_manager import LifespanManager, State +from fastapi_lifespan_manager import State from servicelib.logging_utils import log_catch, log_context from settings_library.postgres import PostgresSettings from sqlalchemy.ext.asyncio import AsyncEngine @@ -13,41 +14,41 @@ _logger = logging.getLogger(__name__) -postgres_lifespan_manager = LifespanManager() - - -class PostgresLifespanStateKeys(str, Enum): +class PostgresLifespanState(str, Enum): POSTGRES_SETTINGS = "postgres_settings" POSTGRES_ASYNC_ENGINE = "postgres.async_engine" class PostgresConfigurationError(LifespanOnStartupError): - msg_template = "Invalid postgres settings [={pg_settings}] on startup. Note that postgres cannot be disabled using settings" + msg_template = "Invalid postgres settings [={settings}] on startup. Note that postgres cannot be disabled using settings" + +def create_input_state(settings: PostgresSettings) -> State: + return {PostgresLifespanState.POSTGRES_SETTINGS: settings} -@postgres_lifespan_manager.add -async def setup_postgres_database(_, state: State) -> AsyncIterator[State]: + +async def postgres_database_lifespan(_, state: State) -> AsyncIterator[State]: with log_context(_logger, logging.INFO, f"{__name__}"): - pg_settings: PostgresSettings | None = state[ - PostgresLifespanStateKeys.POSTGRES_SETTINGS - ] + settings = state[PostgresLifespanState.POSTGRES_SETTINGS] - if pg_settings is None or not isinstance(pg_settings, PostgresSettings): - raise PostgresConfigurationError(pg_settings=pg_settings, module="postgres") + if settings is None or not isinstance(settings, PostgresSettings): + raise PostgresConfigurationError(settings=settings) - assert isinstance(pg_settings, PostgresSettings) # nosec + assert isinstance(settings, PostgresSettings) # nosec + # connect to database async_engine: AsyncEngine = await create_async_engine_and_database_ready( - pg_settings + settings ) try: + yield { - PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE: async_engine, + PostgresLifespanState.POSTGRES_ASYNC_ENGINE: async_engine, } finally: with log_catch(_logger, reraise=False): - await async_engine.dispose() + await asyncio.wait_for(async_engine.dispose(), timeout=10) diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index e0dc1436ffb..a89b64603f2 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -56,7 +56,7 @@ async def cache_lifespan(app: FastAPI) -> AsyncIterator[State]: @pytest.fixture -def postgres_lifespan_mng() -> LifespanManager: +def postgres_lifespan() -> LifespanManager: lifespan_manager = LifespanManager() @lifespan_manager.add @@ -70,8 +70,8 @@ async def _setup_postgres_async_engine(_, state: State) -> AsyncIterator[State]: with log_context(logging.INFO, "postgres_async_engine"): # pass state to children - state["postgres"].update(aengine="Some Async Engine") - yield state + current = state["postgres"] + yield {"postgres": {"aengine": "Some Async Engine", **current}} return lifespan_manager @@ -94,13 +94,13 @@ async def _setup_rabbitmq(app: FastAPI) -> AsyncIterator[State]: async def test_app_lifespan_composition( - postgres_lifespan_mng: LifespanManager, rabbitmq_lifespan: LifespanManager + postgres_lifespan: LifespanManager, rabbitmq_lifespan: LifespanManager ): # The app has its own database and rpc-server to initialize # this is how you connect the lifespans pre-defined in servicelib - @postgres_lifespan_mng.add - async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: + @postgres_lifespan.add + async def database_lifespan(app: FastAPI, state: State) -> AsyncIterator[State]: with log_context(logging.INFO, "app database"): assert state["postgres"] == { @@ -119,7 +119,7 @@ async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: assert app.state.database_engine @rabbitmq_lifespan.add - async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: + async def rpc_service_lifespan(app: FastAPI, state: State) -> AsyncIterator[State]: with log_context(logging.INFO, "app rpc-server"): assert "rabbitmq_rpc_server" in state @@ -129,7 +129,7 @@ async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: # Composes lifepans app_lifespan = LifespanManager() - app_lifespan.include(postgres_lifespan_mng) + app_lifespan.include(postgres_lifespan) app_lifespan.include(rabbitmq_lifespan) app = FastAPI(lifespan=app_lifespan) @@ -165,7 +165,7 @@ async def setup_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: @pytest.fixture -def failing_lifespan_manager(mocker: MockerFixture): +def failing_lifespan_manager(mocker: MockerFixture) -> dict[str, Any]: startup_step = mocker.MagicMock() shutdown_step = mocker.MagicMock() handle_error = mocker.MagicMock() @@ -174,8 +174,8 @@ def raise_error(): msg = "failing module" raise RuntimeError(msg) - async def setup_failing_on_startup(app: FastAPI) -> AsyncIterator[State]: - _name = setup_failing_on_startup.__name__ + async def lifespan_failing_on_startup(app: FastAPI) -> AsyncIterator[State]: + _name = lifespan_failing_on_startup.__name__ with log_context(logging.INFO, _name): try: @@ -187,8 +187,8 @@ async def setup_failing_on_startup(app: FastAPI) -> AsyncIterator[State]: yield {} shutdown_step(_name) - async def setup_failing_on_shutdown(app: FastAPI) -> AsyncIterator[State]: - _name = setup_failing_on_shutdown.__name__ + async def lifespan_failing_on_shutdown(app: FastAPI) -> AsyncIterator[State]: + _name = lifespan_failing_on_shutdown.__name__ with log_context(logging.INFO, _name): startup_step(_name) @@ -204,8 +204,8 @@ async def setup_failing_on_shutdown(app: FastAPI) -> AsyncIterator[State]: "startup_step": startup_step, "shutdown_step": shutdown_step, "handle_error": handle_error, - "setup_failing_on_startup": setup_failing_on_startup, - "setup_failing_on_shutdown": setup_failing_on_shutdown, + "lifespan_failing_on_startup": lifespan_failing_on_startup, + "lifespan_failing_on_shutdown": lifespan_failing_on_shutdown, } @@ -213,7 +213,7 @@ async def test_app_lifespan_with_error_on_startup( failing_lifespan_manager: dict[str, Any], ): app_lifespan = LifespanManager() - app_lifespan.add(failing_lifespan_manager["setup_failing_on_startup"]) + app_lifespan.add(failing_lifespan_manager["lifespan_failing_on_startup"]) app = FastAPI(lifespan=app_lifespan) with pytest.raises(LifespanOnStartupError) as err_info: @@ -225,8 +225,8 @@ async def test_app_lifespan_with_error_on_startup( assert not failing_lifespan_manager["startup_step"].called assert not failing_lifespan_manager["shutdown_step"].called assert exception.error_context() == { - "module": "setup_failing_on_startup", - "message": "Failed during startup of setup_failing_on_startup", + "module": "lifespan_failing_on_startup", + "message": "Failed during startup of lifespan_failing_on_startup", "code": "RuntimeError.LifespanError.LifespanOnStartupError", } @@ -235,7 +235,7 @@ async def test_app_lifespan_with_error_on_shutdown( failing_lifespan_manager: dict[str, Any], ): app_lifespan = LifespanManager() - app_lifespan.add(failing_lifespan_manager["setup_failing_on_shutdown"]) + app_lifespan.add(failing_lifespan_manager["lifespan_failing_on_shutdown"]) app = FastAPI(lifespan=app_lifespan) with pytest.raises(LifespanOnShutdownError) as err_info: @@ -247,7 +247,7 @@ async def test_app_lifespan_with_error_on_shutdown( assert failing_lifespan_manager["startup_step"].called assert not failing_lifespan_manager["shutdown_step"].called assert exception.error_context() == { - "module": "setup_failing_on_shutdown", - "message": "Failed during shutdown of setup_failing_on_shutdown", + "module": "lifespan_failing_on_shutdown", + "message": "Failed during shutdown of lifespan_failing_on_shutdown", "code": "RuntimeError.LifespanError.LifespanOnShutdownError", } diff --git a/packages/service-library/tests/fastapi/test_postgres_lifespan.py b/packages/service-library/tests/fastapi/test_postgres_lifespan.py index 0635daa0a2d..0c656c37187 100644 --- a/packages/service-library/tests/fastapi/test_postgres_lifespan.py +++ b/packages/service-library/tests/fastapi/test_postgres_lifespan.py @@ -18,8 +18,8 @@ from pytest_simcore.helpers.typing_env import EnvVarsDict from servicelib.fastapi.postgres_lifespan import ( PostgresConfigurationError, - PostgresLifespanStateKeys, - postgres_lifespan_manager, + PostgresLifespanState, + postgres_database_lifespan, ) from settings_library.application import BaseApplicationSettings from settings_library.postgres import PostgresSettings @@ -58,11 +58,11 @@ async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: app.state.settings = AppSettings.create_from_envs() yield { - PostgresLifespanStateKeys.POSTGRES_SETTINGS: app.state.settings.CATALOG_POSTGRES + PostgresLifespanState.POSTGRES_SETTINGS: app.state.settings.CATALOG_POSTGRES } async def my_database_setup(app: FastAPI, state: State) -> AsyncIterator[State]: - app.state.my_db_engine = state[PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE] + app.state.my_db_engine = state[PostgresLifespanState.POSTGRES_ASYNC_ENGINE] yield {} @@ -70,13 +70,14 @@ async def my_database_setup(app: FastAPI, state: State) -> AsyncIterator[State]: app_lifespan = LifespanManager() app_lifespan.add(my_app_settings) - postgres_lifespan_manager.add(my_database_setup) - app_lifespan.include(postgres_lifespan_manager) + # potsgres + app_lifespan.add(postgres_database_lifespan) + app_lifespan.add(my_database_setup) return app_lifespan -async def test_setup_postgres_database_in_an_app( +async def test_lifespan_postgres_database_in_an_app( is_pdb_enabled: bool, app_environment: EnvVarsDict, mock_create_async_engine_and_database_ready: MockType, @@ -97,14 +98,14 @@ async def test_setup_postgres_database_in_an_app( # Verify that the async engine is in the lifespan manager state assert ( - PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE + PostgresLifespanState.POSTGRES_ASYNC_ENGINE in asgi_manager._state # noqa: SLF001 ) assert app.state.my_db_engine assert ( app.state.my_db_engine == asgi_manager._state[ # noqa: SLF001 - PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE + PostgresLifespanState.POSTGRES_ASYNC_ENGINE ] ) @@ -118,20 +119,20 @@ async def test_setup_postgres_database_in_an_app( async_engine.dispose.assert_called_once() -async def test_setup_postgres_database_dispose_engine_on_failure( +async def test_lifespan_postgres_database_dispose_engine_on_failure( is_pdb_enabled: bool, app_environment: EnvVarsDict, mock_create_async_engine_and_database_ready: MockType, app_lifespan: LifespanManager, ): - expected_msg = "my_faulty_setup error" + expected_msg = "my_faulty_lifespan error" def raise_error(): raise RuntimeError(expected_msg) @app_lifespan.add - async def my_faulty_setup(app: FastAPI, state: State) -> AsyncIterator[State]: - assert PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE in state + async def my_faulty_lifespan(app: FastAPI, state: State) -> AsyncIterator[State]: + assert PostgresLifespanState.POSTGRES_ASYNC_ENGINE in state raise_error() yield {} @@ -154,14 +155,13 @@ async def test_setup_postgres_database_with_empty_pg_settings( is_pdb_enabled: bool, ): async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: - yield {PostgresLifespanStateKeys.POSTGRES_SETTINGS: None} + yield {PostgresLifespanState.POSTGRES_SETTINGS: None} - app_lifespan_manager = LifespanManager() - app_lifespan_manager.add(my_app_settings) - - app_lifespan_manager.include(postgres_lifespan_manager) + app_lifespan = LifespanManager() + app_lifespan.add(my_app_settings) + app_lifespan.add(postgres_database_lifespan) - app = FastAPI(lifespan=app_lifespan_manager) + app = FastAPI(lifespan=app_lifespan) with pytest.raises(PostgresConfigurationError, match="postgres cannot be disabled"): async with ASGILifespanManager( diff --git a/services/catalog/src/simcore_service_catalog/api/rpc/routes.py b/services/catalog/src/simcore_service_catalog/api/rpc/routes.py index f9960271d2c..3c2310c2680 100644 --- a/services/catalog/src/simcore_service_catalog/api/rpc/routes.py +++ b/services/catalog/src/simcore_service_catalog/api/rpc/routes.py @@ -11,7 +11,7 @@ _logger = logging.getLogger(__name__) -async def setup_rpc_api_routes(app: FastAPI) -> AsyncIterator[State]: +async def rpc_api_lifespan(app: FastAPI) -> AsyncIterator[State]: rpc_server = get_rabbitmq_rpc_server(app) await rpc_server.register_router(_services.router, CATALOG_RPC_NAMESPACE, app) try: diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py index 0db5debbcb8..dfd5365b562 100644 --- a/services/catalog/src/simcore_service_catalog/core/background_tasks.py +++ b/services/catalog/src/simcore_service_catalog/core/background_tasks.py @@ -236,7 +236,7 @@ async def stop_registry_sync_task(app: FastAPI) -> None: _logger.info("registry syncing task stopped") -async def setup_background_task(app: FastAPI) -> AsyncIterator[State]: +async def background_task_lifespan(app: FastAPI) -> AsyncIterator[State]: await start_registry_sync_task(app) try: yield {} diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 267ccdd20f7..44d3bf9dd7d 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -4,21 +4,20 @@ from fastapi import FastAPI from fastapi_lifespan_manager import LifespanManager, State from servicelib.fastapi.postgres_lifespan import ( - PostgresLifespanStateKeys, - postgres_lifespan_manager, + PostgresLifespanState, + postgres_database_lifespan, ) from servicelib.fastapi.prometheus_instrumentation import ( lifespan_prometheus_instrumentation, ) -from servicelib.fastapi.tracing import initialize_tracing - -from .._meta import APP_FINISHED_BANNER_MSG, APP_NAME, APP_STARTED_BANNER_MSG -from ..api.rpc.routes import setup_rpc_api_routes -from ..db.events import setup_database -from ..services.director import setup_director -from ..services.function_services import setup_function_services -from ..services.rabbitmq import setup_rabbitmq -from .background_tasks import setup_background_task + +from .._meta import APP_FINISHED_BANNER_MSG, APP_STARTED_BANNER_MSG +from ..api.rpc.routes import rpc_api_lifespan +from ..db.events import database_lifespan +from ..services.director import director_lifespan +from ..services.function_services import function_services_lifespan +from ..services.rabbitmq import rabbitmq_lifespan +from .background_tasks import background_task_lifespan from .settings import ApplicationSettings _logger = logging.getLogger(__name__) @@ -38,11 +37,8 @@ async def _setup_app(app: FastAPI) -> AsyncIterator[State]: settings: ApplicationSettings = app.state.settings - if settings.CATALOG_TRACING: - initialize_tracing(app, settings.CATALOG_TRACING, APP_NAME) - yield { - PostgresLifespanStateKeys.POSTGRES_SETTINGS: settings.CATALOG_POSTGRES, + PostgresLifespanState.POSTGRES_SETTINGS: settings.CATALOG_POSTGRES, "prometheus_instrumentation_enabled": settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED, } @@ -66,23 +62,23 @@ def create_app_lifespan(): # WARNING: order matters # - postgres lifespan - postgres_lifespan_manager.add(setup_database) - app_lifespan.include(postgres_lifespan_manager) + app_lifespan.add(postgres_database_lifespan) + app_lifespan.add(database_lifespan) # - rabbitmq lifespan - app_lifespan.add(setup_rabbitmq) + app_lifespan.add(rabbitmq_lifespan) # - rpc api routes lifespan - app_lifespan.add(setup_rpc_api_routes) + app_lifespan.add(rpc_api_lifespan) # - director lifespan - app_lifespan.add(setup_director) + app_lifespan.add(director_lifespan) # - function services lifespan - app_lifespan.add(setup_function_services) + app_lifespan.add(function_services_lifespan) # - background task lifespan - app_lifespan.add(setup_background_task) + app_lifespan.add(background_task_lifespan) # - prometheus instrumentation lifespan app_lifespan.add(_setup_prometheus_instrumentation_adapter) diff --git a/services/catalog/src/simcore_service_catalog/db/events.py b/services/catalog/src/simcore_service_catalog/db/events.py index 82702fd0f78..950f67e373f 100644 --- a/services/catalog/src/simcore_service_catalog/db/events.py +++ b/services/catalog/src/simcore_service_catalog/db/events.py @@ -3,15 +3,15 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State -from servicelib.fastapi.postgres_lifespan import PostgresLifespanStateKeys +from servicelib.fastapi.postgres_lifespan import PostgresLifespanState from .repositories.products import ProductsRepository _logger = logging.getLogger(__name__) -async def setup_database(app: FastAPI, state: State) -> AsyncIterator[State]: - app.state.engine = state[PostgresLifespanStateKeys.POSTGRES_ASYNC_ENGINE] +async def database_lifespan(app: FastAPI, state: State) -> AsyncIterator[State]: + app.state.engine = state[PostgresLifespanState.POSTGRES_ASYNC_ENGINE] repo = ProductsRepository(db_engine=app.state.engine) diff --git a/services/catalog/src/simcore_service_catalog/services/director.py b/services/catalog/src/simcore_service_catalog/services/director.py index 5cac7f037c3..7560c622317 100644 --- a/services/catalog/src/simcore_service_catalog/services/director.py +++ b/services/catalog/src/simcore_service_catalog/services/director.py @@ -289,7 +289,7 @@ async def get_service_extras( return TypeAdapter(ServiceExtras).validate_python(result) -async def setup_director(app: FastAPI) -> AsyncIterator[State]: +async def director_lifespan(app: FastAPI) -> AsyncIterator[State]: client: DirectorApi | None = None if settings := app.state.settings.CATALOG_DIRECTOR: diff --git a/services/catalog/src/simcore_service_catalog/services/function_services.py b/services/catalog/src/simcore_service_catalog/services/function_services.py index 9f939ec46fe..c5f326fec4b 100644 --- a/services/catalog/src/simcore_service_catalog/services/function_services.py +++ b/services/catalog/src/simcore_service_catalog/services/function_services.py @@ -34,7 +34,7 @@ def get_function_service(key, version) -> ServiceMetaDataPublished: ) from err -async def setup_function_services(app: FastAPI) -> AsyncIterator[State]: +async def function_services_lifespan(app: FastAPI) -> AsyncIterator[State]: app.state.frontend_services_catalog = [ _as_dict(metadata) for metadata in iter_service_docker_data() ] @@ -48,5 +48,5 @@ async def setup_function_services(app: FastAPI) -> AsyncIterator[State]: __all__: tuple[str, ...] = ( "get_function_service", "is_function_service", - "setup_function_services", + "function_services_lifespan", ) diff --git a/services/catalog/src/simcore_service_catalog/services/rabbitmq.py b/services/catalog/src/simcore_service_catalog/services/rabbitmq.py index c2b02259dd5..ba46173b647 100644 --- a/services/catalog/src/simcore_service_catalog/services/rabbitmq.py +++ b/services/catalog/src/simcore_service_catalog/services/rabbitmq.py @@ -17,7 +17,7 @@ def get_rabbitmq_settings(app: FastAPI) -> RabbitSettings: return settings -async def setup_rabbitmq(app: FastAPI) -> AsyncIterator[State]: +async def rabbitmq_lifespan(app: FastAPI) -> AsyncIterator[State]: settings: RabbitSettings = get_rabbitmq_settings(app) await wait_till_rabbitmq_responsive(settings.dsn) From f7c7f5d6ccbe84210d6548e084c1c9814400cba0 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 14:49:00 +0200 Subject: [PATCH 24/29] @sanderegg review: renames --- .../core/application.py | 5 +++ .../simcore_service_catalog/core/events.py | 44 ++++++++++--------- services/catalog/tests/unit/conftest.py | 4 +- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/application.py b/services/catalog/src/simcore_service_catalog/core/application.py index 831030f4cfd..15762c09226 100644 --- a/services/catalog/src/simcore_service_catalog/core/application.py +++ b/services/catalog/src/simcore_service_catalog/core/application.py @@ -8,11 +8,13 @@ from servicelib.fastapi.prometheus_instrumentation import ( initialize_prometheus_instrumentation, ) +from servicelib.fastapi.tracing import initialize_tracing from starlette.middleware.base import BaseHTTPMiddleware from .._meta import ( API_VERSION, API_VTAG, + APP_NAME, PROJECT_NAME, SUMMARY, ) @@ -62,6 +64,9 @@ def create_app() -> FastAPI: app.state.settings = settings # MIDDLEWARES + if settings.CATALOG_TRACING: + initialize_tracing(app, settings.CATALOG_TRACING, APP_NAME) + if settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED: initialize_prometheus_instrumentation(app) diff --git a/services/catalog/src/simcore_service_catalog/core/events.py b/services/catalog/src/simcore_service_catalog/core/events.py index 44d3bf9dd7d..097a4ab4113 100644 --- a/services/catalog/src/simcore_service_catalog/core/events.py +++ b/services/catalog/src/simcore_service_catalog/core/events.py @@ -23,18 +23,23 @@ _logger = logging.getLogger(__name__) -def flush_started_banner() -> None: +def _flush_started_banner() -> None: # WARNING: this function is spied in the tests print(APP_STARTED_BANNER_MSG, flush=True) # noqa: T201 -def flush_finished_banner() -> None: +def _flush_finished_banner() -> None: + # WARNING: this function is spied in the tests print(APP_FINISHED_BANNER_MSG, flush=True) # noqa: T201 -async def _setup_app(app: FastAPI) -> AsyncIterator[State]: - flush_started_banner() +async def _banners_lifespan(_) -> AsyncIterator[State]: + _flush_started_banner() + yield {} + _flush_finished_banner() + +async def _main_lifespan(app: FastAPI) -> AsyncIterator[State]: settings: ApplicationSettings = app.state.settings yield { @@ -42,45 +47,42 @@ async def _setup_app(app: FastAPI) -> AsyncIterator[State]: "prometheus_instrumentation_enabled": settings.CATALOG_PROMETHEUS_INSTRUMENTATION_ENABLED, } - flush_finished_banner() - -async def _setup_prometheus_instrumentation_adapter( +async def _prometheus_instrumentation_lifespan( app: FastAPI, state: State ) -> AsyncIterator[State]: - enabled = state.get("prometheus_instrumentation_enabled", False) - if enabled: + if state.get("prometheus_instrumentation_enabled", False): async for prometheus_state in lifespan_prometheus_instrumentation(app): yield prometheus_state def create_app_lifespan(): - # app lifespan - app_lifespan = LifespanManager() - app_lifespan.add(_setup_app) - # WARNING: order matters + app_lifespan = LifespanManager() + app_lifespan.add(_main_lifespan) - # - postgres lifespan + # - postgres app_lifespan.add(postgres_database_lifespan) app_lifespan.add(database_lifespan) - # - rabbitmq lifespan + # - rabbitmq app_lifespan.add(rabbitmq_lifespan) - # - rpc api routes lifespan + # - rpc api routes app_lifespan.add(rpc_api_lifespan) - # - director lifespan + # - director app_lifespan.add(director_lifespan) - # - function services lifespan + # - function services app_lifespan.add(function_services_lifespan) - # - background task lifespan + # - background task app_lifespan.add(background_task_lifespan) - # - prometheus instrumentation lifespan - app_lifespan.add(_setup_prometheus_instrumentation_adapter) + # - prometheus instrumentation + app_lifespan.add(_prometheus_instrumentation_lifespan) + + app_lifespan.add(_banners_lifespan) return app_lifespan diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 7b11d57be10..9db117f4104 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -118,11 +118,11 @@ def spy_app(mocker: MockerFixture) -> AppLifeSpanSpyTargets: return AppLifeSpanSpyTargets( on_startup=mocker.spy( simcore_service_catalog.core.events, - "flush_started_banner", + "_flush_started_banner", ), on_shutdown=mocker.spy( simcore_service_catalog.core.events, - "flush_finished_banner", + "_flush_finished_banner", ), ) From b088016d52ff9e00a53942af91259238c18dad34 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 15:39:19 +0200 Subject: [PATCH 25/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Rename=20lifespan?= =?UTF-8?q?=20mocks=20for=20clarity=20and=20consistency=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/catalog/tests/unit/conftest.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 9db117f4104..b54f2ce7b6f 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -217,7 +217,10 @@ def service_caching_disabled(monkeypatch: pytest.MonkeyPatch) -> None: @pytest.fixture def postgres_setup_disabled(mocker: MockerFixture): - mocker.patch.object(simcore_service_catalog.core.events, "postgres_lifespan") + mocker.patch.object( + simcore_service_catalog.core.events, "postgres_database_lifespan" + ) + mocker.patch.object(simcore_service_catalog.core.events, "database_lifespan") @pytest.fixture @@ -241,7 +244,7 @@ async def __aexit__(self, exc_type, exc_value, traceback): mocker.patch.object( simcore_service_catalog.core.events, - "setup_background_task", + "background_task_lifespan", return_value=MockedBackgroundTaskContextManager(), ) @@ -254,8 +257,8 @@ async def __aexit__(self, exc_type, exc_value, traceback): @pytest.fixture def rabbitmq_and_rpc_setup_disabled(mocker: MockerFixture): # The following services are affected if rabbitmq is not in place - mocker.patch.object(simcore_service_catalog.core.events, "setup_rabbitmq") - mocker.patch.object(simcore_service_catalog.core.events, "setup_rpc_api_routes") + mocker.patch.object(simcore_service_catalog.core.events, "rabbitmq_lifespan") + mocker.patch.object(simcore_service_catalog.core.events, "rpc_api_lifespan") @pytest.fixture From 530f4dfe5e93083caa56041e05735cf42ae7fb3e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 15:42:05 +0200 Subject: [PATCH 26/29] minor --- services/catalog/tests/unit/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index b54f2ce7b6f..120b9b20689 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -200,7 +200,7 @@ async def aclient( headers={"Content-Type": "application/json"}, transport=httpx.ASGITransport(app=app), ) as acli: - assert isinstance(acli._transport, httpx.ASGITransport) + assert isinstance(acli._transport, httpx.ASGITransport) # noqa: SLF001 assert spy_app.on_startup.call_count == 1 assert spy_app.on_shutdown.call_count == 0 From 1fd74536845048eddd8b7daf7db5077999be0e51 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 16:28:52 +0200 Subject: [PATCH 27/29] fixes fixture --- services/catalog/tests/unit/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/tests/unit/conftest.py b/services/catalog/tests/unit/conftest.py index 120b9b20689..02a027f63f5 100644 --- a/services/catalog/tests/unit/conftest.py +++ b/services/catalog/tests/unit/conftest.py @@ -275,7 +275,7 @@ async def rpc_client( @pytest.fixture def director_setup_disabled(mocker: MockerFixture) -> None: - mocker.patch.object(simcore_service_catalog.core.events, "setup_director") + mocker.patch.object(simcore_service_catalog.core.events, "director_lifespan") @pytest.fixture From 2145c096d78516d46efa79dfa9bfbea33729a7bf Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 16:44:33 +0200 Subject: [PATCH 28/29] =?UTF-8?q?=E2=9C=A8=20Refactor:=20Update=20service?= =?UTF-8?q?=20specifications=20and=20remove=20redundant=20null=20settings?= =?UTF-8?q?=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/simcore_service_catalog/core/settings.py | 10 +++++----- services/catalog/tests/unit/test_services_director.py | 1 - services/catalog/tests/unit/test_services_manifest.py | 1 - 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/settings.py b/services/catalog/src/simcore_service_catalog/core/settings.py index eba2176bc81..5581bf4ba99 100644 --- a/services/catalog/src/simcore_service_catalog/core/settings.py +++ b/services/catalog/src/simcore_service_catalog/core/settings.py @@ -46,9 +46,9 @@ def base_url(self) -> str: ) -_DEFAULT_SERVICE_SPECIFICATIONS: Final[ - ServiceSpecifications -] = ServiceSpecifications.model_validate({}) +_DEFAULT_SERVICE_SPECIFICATIONS: Final[ServiceSpecifications] = ( + ServiceSpecifications.model_validate({}) +) class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings): @@ -88,7 +88,7 @@ class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings): ] = False CATALOG_POSTGRES: Annotated[ - PostgresSettings | None, + PostgresSettings, Field(json_schema_extra={"auto_default_from_env": True}), ] @@ -102,7 +102,7 @@ class ApplicationSettings(BaseApplicationSettings, MixinLoggingSettings): ] CATALOG_DIRECTOR: Annotated[ - DirectorSettings | None, + DirectorSettings, Field(json_schema_extra={"auto_default_from_env": True}), ] diff --git a/services/catalog/tests/unit/test_services_director.py b/services/catalog/tests/unit/test_services_director.py index 04a47cad08c..94998f74749 100644 --- a/services/catalog/tests/unit/test_services_director.py +++ b/services/catalog/tests/unit/test_services_director.py @@ -27,7 +27,6 @@ def app_environment( monkeypatch, { **app_environment, - "CATALOG_POSTGRES": "null", # disable postgres "SC_BOOT_MODE": "local-development", }, ) diff --git a/services/catalog/tests/unit/test_services_manifest.py b/services/catalog/tests/unit/test_services_manifest.py index 5dc354a21eb..67b9b03e2e3 100644 --- a/services/catalog/tests/unit/test_services_manifest.py +++ b/services/catalog/tests/unit/test_services_manifest.py @@ -26,7 +26,6 @@ def app_environment( monkeypatch, { **app_environment, - "CATALOG_POSTGRES": "null", # disable postgres "SC_BOOT_MODE": "local-development", }, ) From 0ccb68198c9aefca9fb1d6eaf938d328125d174b Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 8 Apr 2025 16:49:06 +0200 Subject: [PATCH 29/29] fixes test --- services/catalog/tests/unit/with_dbs/test_db_repositories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/catalog/tests/unit/with_dbs/test_db_repositories.py b/services/catalog/tests/unit/with_dbs/test_db_repositories.py index 8618a67c25e..40a4f081764 100644 --- a/services/catalog/tests/unit/with_dbs/test_db_repositories.py +++ b/services/catalog/tests/unit/with_dbs/test_db_repositories.py @@ -487,7 +487,7 @@ async def test_get_service_history_page( user_id: UserID, ): # inject services with multiple versions - service_key = "simcore/services/dynamic/test-service" + service_key = "simcore/services/dynamic/test-some-service" num_versions = 10 release_versions = [