From fd9055f647444cdd992907e0d63cf22c61ec2db9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 16 Apr 2025 20:37:28 +0200 Subject: [PATCH 1/9] =?UTF-8?q?=E2=9C=A8=20Enhance=20lifespan=20error=20ha?= =?UTF-8?q?ndling=20and=20add=20lifespan=20call=20tracking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/lifespan_utils.py | 36 +++++++++++++++++- .../tests/fastapi/test_lifespan_utils.py | 38 +++++++++++++++++-- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py index 8b16f5bec194..e8605e66ccdd 100644 --- a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py +++ b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py @@ -1,12 +1,44 @@ +from typing import Final + from common_library.errors_classes import OsparcErrorMixin +from fastapi import FastAPI +from fastapi_lifespan_manager import State class LifespanError(OsparcErrorMixin, RuntimeError): ... class LifespanOnStartupError(LifespanError): - msg_template = "Failed during startup of {module}" + msg_template = "Failed during startup of {lifespan_name}" class LifespanOnShutdownError(LifespanError): - msg_template = "Failed during shutdown of {module}" + msg_template = "Failed during shutdown of {lifespan_name}" + + +class LifespanAlreadyCalledError(LifespanError): + msg_template = "The lifespan '{lifespan_name}' has already been called." + + +_CALLED_LIFESPANS_KEY: Final[str] = "_CALLED_LIFESPANS" + + +def is_lifespan_called(state: State, lifespan_name: str) -> bool: + called_lifespans = state.get(_CALLED_LIFESPANS_KEY, set()) + return lifespan_name in called_lifespans + + +def record_lifespan_called_once(state: State, lifespan_name: str) -> State: + """Validates if a lifespan has already been called and records it in the state. + Raises LifespanAlreadyCalledError if the lifespan has already been called. + """ + assert not isinstance( # nosec + state, FastAPI + ), "TIP: lifespan func has (app, state) positional arguments" + + if is_lifespan_called(state, lifespan_name): + raise LifespanAlreadyCalledError(lifespan_name=lifespan_name) + + called_lifespans = state.get(_CALLED_LIFESPANS_KEY, set()) + called_lifespans.add(lifespan_name) + return {_CALLED_LIFESPANS_KEY: called_lifespans} diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index 0c3d2767d2a1..e247cf2a19b1 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -16,8 +16,10 @@ from pytest_mock import MockerFixture from pytest_simcore.helpers.logging_tools import log_context from servicelib.fastapi.lifespan_utils import ( + LifespanAlreadyCalledError, LifespanOnShutdownError, LifespanOnStartupError, + record_lifespan_called_once, ) @@ -186,7 +188,7 @@ async def lifespan_failing_on_startup(app: FastAPI) -> AsyncIterator[State]: startup_step(_name) except RuntimeError as exc: handle_error(_name, exc) - raise LifespanOnStartupError(module=_name) from exc + raise LifespanOnStartupError(lifespan_name=_name) from exc yield {} shutdown_step(_name) @@ -201,7 +203,7 @@ async def lifespan_failing_on_shutdown(app: FastAPI) -> AsyncIterator[State]: shutdown_step(_name) except RuntimeError as exc: handle_error(_name, exc) - raise LifespanOnShutdownError(module=_name) from exc + raise LifespanOnShutdownError(lifespan_name=_name) from exc return { "startup_step": startup_step, @@ -228,7 +230,7 @@ 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": "lifespan_failing_on_startup", + "lifespan_name": "lifespan_failing_on_startup", "message": "Failed during startup of lifespan_failing_on_startup", "code": "RuntimeError.LifespanError.LifespanOnStartupError", } @@ -250,7 +252,35 @@ 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": "lifespan_failing_on_shutdown", + "lifespan_name": "lifespan_failing_on_shutdown", "message": "Failed during shutdown of lifespan_failing_on_shutdown", "code": "RuntimeError.LifespanError.LifespanOnShutdownError", } + + +async def test_lifespan_called_more_than_once(is_pdb_enabled: bool): + state = {} + + app_lifespan = LifespanManager() + + @app_lifespan.add + async def _one(_, state: State) -> AsyncIterator[State]: + called_state = record_lifespan_called_once(state, "test_lifespan_one") + yield {"other": 0, **called_state} + + @app_lifespan.add + async def _two(_, state: State) -> AsyncIterator[State]: + called_state = record_lifespan_called_once(state, "test_lifespan_two") + yield {"something": 0, **called_state} + + app_lifespan.add(_one) # added "by mistake" + + with pytest.raises(LifespanAlreadyCalledError) as err_info: + async with ASGILifespanManager( + FastAPI(lifespan=app_lifespan), + startup_timeout=None if is_pdb_enabled else 10, + shutdown_timeout=None if is_pdb_enabled else 10, + ): + ... + + assert err_info.value.lifespan_name == "test_lifespan_one" From 16cd4a1419aef630c7a11053eda6224b0af742b9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 16 Apr 2025 20:38:24 +0200 Subject: [PATCH 2/9] =?UTF-8?q?=E2=9C=A8=20Add=20JSON=20schema=20examples?= =?UTF-8?q?=20to=20RedisSettings=20model=20configuration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/settings_library/redis.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/settings-library/src/settings_library/redis.py b/packages/settings-library/src/settings_library/redis.py index 6e21968d0438..3e3548ef3945 100644 --- a/packages/settings-library/src/settings_library/redis.py +++ b/packages/settings-library/src/settings_library/redis.py @@ -2,6 +2,7 @@ from pydantic.networks import RedisDsn from pydantic.types import SecretStr +from pydantic_settings import SettingsConfigDict from .base import BaseCustomSettings from .basic_types import PortInt @@ -45,3 +46,18 @@ def build_redis_dsn(self, db_index: RedisDatabase) -> str: path=f"{db_index}", ) ) + + model_config = SettingsConfigDict( + json_schema_extra={ + "examples": [ + # minimal required + { + "REDIS_SECURE": "0", + "REDIS_HOST": "localhost", + "REDIS_PORT": "6379", + "REDIS_USER": "user", + "REDIS_PASSWORD": "foobar", # NOSONAR + } + ], + } + ) From 9fed19b6fa507b014c728afbd696d3289b016dff Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 16 Apr 2025 20:38:36 +0200 Subject: [PATCH 3/9] =?UTF-8?q?=E2=9C=A8=20Implement=20Redis=20lifespan=20?= =?UTF-8?q?management=20and=20validation=20in=20FastAPI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/redis_lifespan.py | 69 ++++++++++ .../tests/fastapi/test_redis_lifespan.py | 130 ++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 packages/service-library/src/servicelib/fastapi/redis_lifespan.py create mode 100644 packages/service-library/tests/fastapi/test_redis_lifespan.py diff --git a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py new file mode 100644 index 000000000000..3424da575176 --- /dev/null +++ b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py @@ -0,0 +1,69 @@ +import asyncio +import logging +from collections.abc import AsyncIterator +from typing import Annotated + +from fastapi import FastAPI +from fastapi_lifespan_manager import State +from pydantic import BaseModel, ConfigDict, StringConstraints, ValidationError +from servicelib.logging_utils import log_catch, log_context +from settings_library.redis import RedisDatabase, RedisSettings + +from ..redis import RedisClientSDK +from .lifespan_utils import LifespanOnStartupError, record_lifespan_called_once + +_logger = logging.getLogger(__name__) + + +class RedisConfigurationError(LifespanOnStartupError): + msg_template = "Invalid redis config on startup : {validation_error}" + + +class RedisLifespanState(BaseModel): + REDIS_SETTINGS: RedisSettings + REDIS_CLIENT_NAME: Annotated[str, StringConstraints(min_length=3, max_length=32)] + REDIS_CLIENT_DB: RedisDatabase + + model_config = ConfigDict( + extra="allow", + arbitrary_types_allowed=True, # RedisClientSDK has some arbitrary types and this class will never be serialized + ) + + +async def redis_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[State]: + with log_context(_logger, logging.INFO, f"{__name__}"): + + # Check if lifespan has already been called + called_state = record_lifespan_called_once(state, "redis_database_lifespan") + + # Validate input state + try: + redis_state = RedisLifespanState.model_validate(state) + redis_dsn_with_secrets = redis_state.REDIS_SETTINGS.build_redis_dsn( + redis_state.REDIS_CLIENT_DB + ) + except ValidationError as exc: + raise RedisConfigurationError(validation_error=exc, state=state) from exc + + # Setup client + with log_context( + _logger, + logging.INFO, + f"Creating redis client with name={redis_state.REDIS_CLIENT_NAME}", + ): + redis_client = RedisClientSDK( + redis_dsn_with_secrets, + client_name=redis_state.REDIS_CLIENT_NAME, + ) + + try: + yield {"REDIS_CLIENT_SDK": redis_client, **called_state} + finally: + # Teardown client + if redis_client: + with log_catch(_logger, reraise=False): + await asyncio.wait_for( + redis_client.shutdown(), + # NOTE: shutdown already has a _HEALTHCHECK_TASK_TIMEOUT_S of 10s + timeout=20, + ) diff --git a/packages/service-library/tests/fastapi/test_redis_lifespan.py b/packages/service-library/tests/fastapi/test_redis_lifespan.py new file mode 100644 index 000000000000..9095a271d215 --- /dev/null +++ b/packages/service-library/tests/fastapi/test_redis_lifespan.py @@ -0,0 +1,130 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +from collections.abc import AsyncIterator +from typing import Annotated, Any + +import pytest +import servicelib.fastapi.redis_lifespan +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.redis_lifespan import ( + RedisConfigurationError, + RedisLifespanState, + redis_database_lifespan, +) +from settings_library.application import BaseApplicationSettings +from settings_library.redis import RedisDatabase, RedisSettings + + +@pytest.fixture +def mock_redis_client_sdk(mocker: MockerFixture) -> MockType: + return mocker.patch.object( + servicelib.fastapi.redis_lifespan, + "RedisClientSDK", + return_value=mocker.AsyncMock(), + ) + + +@pytest.fixture +def app_environment(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict: + return setenvs_from_dict( + monkeypatch, RedisSettings.model_json_schema()["examples"][0] + ) + + +@pytest.fixture +def app_lifespan( + app_environment: EnvVarsDict, + mock_redis_client_sdk: MockType, +) -> LifespanManager: + assert app_environment + + class AppSettings(BaseApplicationSettings): + CATALOG_REDIS: Annotated[ + RedisSettings, + 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 RedisLifespanState( + REDIS_SETTINGS=app.state.settings.CATALOG_REDIS, + REDIS_CLIENT_NAME="test_client", + REDIS_CLIENT_DB=RedisDatabase.LOCKS, + ).model_dump() + + app_lifespan = LifespanManager() + app_lifespan.add(my_app_settings) + app_lifespan.add(redis_database_lifespan) + + assert not mock_redis_client_sdk.called + + return app_lifespan + + +async def test_lifespan_redis_database_in_an_app( + is_pdb_enabled: bool, + app_environment: EnvVarsDict, + mock_redis_client_sdk: MockType, + app_lifespan: LifespanManager, +): + app = FastAPI(lifespan=app_lifespan) + + 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 Redis client SDK was created + mock_redis_client_sdk.assert_called_once_with( + app.state.settings.CATALOG_REDIS.build_redis_dsn(RedisDatabase.LOCKS), + client_name="test_client", + ) + + # Verify that the Redis client SDK is in the lifespan manager state + assert "REDIS_CLIENT_SDK" in asgi_manager._state # noqa: SLF001 + assert app.state.settings.CATALOG_REDIS + assert ( + asgi_manager._state["REDIS_CLIENT_SDK"] # noqa: SLF001 + == mock_redis_client_sdk.return_value + ) + + # Verify that the Redis client SDK was shut down + redis_client: Any = mock_redis_client_sdk.return_value + redis_client.shutdown.assert_called_once() + + +async def test_lifespan_redis_database_with_invalid_settings( + is_pdb_enabled: bool, +): + async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: + yield {"REDIS_SETTINGS": None} + + app_lifespan = LifespanManager() + app_lifespan.add(my_app_settings) + app_lifespan.add(redis_database_lifespan) + + app = FastAPI(lifespan=app_lifespan) + + with pytest.raises(RedisConfigurationError, match="Invalid redis") as excinfo: + async with ASGILifespanManager( + app, + startup_timeout=None if is_pdb_enabled else 10, + shutdown_timeout=None if is_pdb_enabled else 10, + ): + ... + + exception = excinfo.value + assert isinstance(exception, RedisConfigurationError) + assert exception.validation_error + assert exception.state["REDIS_SETTINGS"] is None From cf8c1f1b3b11f4983dcc0009941af8258fdd3618 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 16 Apr 2025 20:38:47 +0200 Subject: [PATCH 4/9] =?UTF-8?q?=E2=9C=A8=20Add=20lifespan=20call=20trackin?= =?UTF-8?q?g=20for=20PostgreSQL=20database=20in=20FastAPI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/postgres_lifespan.py | 6 +++++- .../src/settings_library/rabbit.py | 15 +++++++++++++++ .../src/settings_library/redis.py | 3 --- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index cc207e6f397a..5d257725062d 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -10,7 +10,7 @@ from sqlalchemy.ext.asyncio import AsyncEngine from ..db_asyncpg_utils import create_async_engine_and_database_ready -from .lifespan_utils import LifespanOnStartupError +from .lifespan_utils import LifespanOnStartupError, record_lifespan_called_once _logger = logging.getLogger(__name__) @@ -32,6 +32,9 @@ async def postgres_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[ with log_context(_logger, logging.INFO, f"{__name__}"): + # Mark lifespan as called + called_state = record_lifespan_called_once(state, "postgres_database_lifespan") + settings = state[PostgresLifespanState.POSTGRES_SETTINGS] if settings is None or not isinstance(settings, PostgresSettings): @@ -48,6 +51,7 @@ async def postgres_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[ yield { PostgresLifespanState.POSTGRES_ASYNC_ENGINE: async_engine, + **called_state, } finally: diff --git a/packages/settings-library/src/settings_library/rabbit.py b/packages/settings-library/src/settings_library/rabbit.py index e2cc2e271cee..1e95bdbec2ea 100644 --- a/packages/settings-library/src/settings_library/rabbit.py +++ b/packages/settings-library/src/settings_library/rabbit.py @@ -2,6 +2,7 @@ from pydantic.networks import AnyUrl from pydantic.types import SecretStr +from pydantic_settings import SettingsConfigDict from .base import BaseCustomSettings from .basic_types import PortInt @@ -33,3 +34,17 @@ def dsn(self) -> str: ) ) return rabbit_dsn + + model_config = SettingsConfigDict( + json_schema_extra={ + "examples": [ + # minimal required + { + "RABBIT_SECURE": "1", + "RABBIT_HOST": "localhost", + "RABBIT_USER": "user", + "RABBIT_PASSWORD": "foobar", # NOSONAR + } + ], + } + ) diff --git a/packages/settings-library/src/settings_library/redis.py b/packages/settings-library/src/settings_library/redis.py index 3e3548ef3945..40dd88aabf98 100644 --- a/packages/settings-library/src/settings_library/redis.py +++ b/packages/settings-library/src/settings_library/redis.py @@ -52,9 +52,6 @@ def build_redis_dsn(self, db_index: RedisDatabase) -> str: "examples": [ # minimal required { - "REDIS_SECURE": "0", - "REDIS_HOST": "localhost", - "REDIS_PORT": "6379", "REDIS_USER": "user", "REDIS_PASSWORD": "foobar", # NOSONAR } From f5c74384ee86521aacb6c429d8b88089975350b1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 16 Apr 2025 21:10:29 +0200 Subject: [PATCH 5/9] =?UTF-8?q?=E2=9C=A8=20Add=20RabbitMQ=20lifespan=20man?= =?UTF-8?q?agement=20and=20validation=20in=20FastAPI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../servicelib/fastapi/rabbitmq_lifespan.py | 48 ++++++ .../src/servicelib/rabbitmq/__init__.py | 3 +- .../src/servicelib/rabbitmq/_client_rpc.py | 19 ++- .../tests/fastapi/test_rabbitmq_lifespan.py | 142 ++++++++++++++++++ 4 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py create mode 100644 packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py diff --git a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py new file mode 100644 index 000000000000..dfc27042de1a --- /dev/null +++ b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py @@ -0,0 +1,48 @@ +import logging +from collections.abc import AsyncIterator + +from fastapi import FastAPI +from fastapi_lifespan_manager import State +from pydantic import BaseModel, ValidationError +from servicelib.logging_utils import log_context +from servicelib.rabbitmq import wait_till_rabbitmq_responsive +from settings_library.rabbit import RabbitSettings + +from .lifespan_utils import LifespanOnStartupError, record_lifespan_called_once + +_logger = logging.getLogger(__name__) + + +class RabbitMQConfigurationError(LifespanOnStartupError): + msg_template = "Invalid RabbitMQ config on startup : {validation_error}" + + +class RabbitMQLifespanState(BaseModel): + RABBIT_SETTINGS: RabbitSettings + + +async def rabbitmq_connectivity_lifespan( + _: FastAPI, state: State +) -> AsyncIterator[State]: + """Ensures RabbitMQ connectivity during lifespan. + + For creating clients, use additional lifespans like rabbitmq_rpc_client_context. + """ + _lifespan_name = f"{__name__}.{rabbitmq_connectivity_lifespan.__name__}" + + with log_context(_logger, logging.INFO, _lifespan_name): + + # Check if lifespan has already been called + called_state = record_lifespan_called_once(state, _lifespan_name) + + # Validate input state + try: + rabbit_state = RabbitMQLifespanState.model_validate(state) + rabbit_dsn_with_secrets = rabbit_state.RABBIT_SETTINGS.dsn + except ValidationError as exc: + raise RabbitMQConfigurationError(validation_error=exc, state=state) from exc + + # Wait for RabbitMQ to be responsive + await wait_till_rabbitmq_responsive(rabbit_dsn_with_secrets) + + yield {"RABBIT_CONNECTIVITY_LIFESPAN_NAME": _lifespan_name, **called_state} diff --git a/packages/service-library/src/servicelib/rabbitmq/__init__.py b/packages/service-library/src/servicelib/rabbitmq/__init__.py index b2e8b6d0b347..ad67487cdd93 100644 --- a/packages/service-library/src/servicelib/rabbitmq/__init__.py +++ b/packages/service-library/src/servicelib/rabbitmq/__init__.py @@ -1,7 +1,7 @@ from models_library.rabbitmq_basic_types import RPCNamespace from ._client import RabbitMQClient -from ._client_rpc import RabbitMQRPCClient +from ._client_rpc import RabbitMQRPCClient, rabbitmq_rpc_client_context from ._constants import BIND_TO_ALL_TOPICS, RPC_REQUEST_DEFAULT_TIMEOUT_S from ._errors import ( RemoteMethodNotRegisteredError, @@ -28,6 +28,7 @@ "RabbitMQRPCClient", "RemoteMethodNotRegisteredError", "is_rabbitmq_responsive", + "rabbitmq_rpc_client_context", "wait_till_rabbitmq_responsive", ) diff --git a/packages/service-library/src/servicelib/rabbitmq/_client_rpc.py b/packages/service-library/src/servicelib/rabbitmq/_client_rpc.py index e34fc874a54f..53d9f1326585 100644 --- a/packages/service-library/src/servicelib/rabbitmq/_client_rpc.py +++ b/packages/service-library/src/servicelib/rabbitmq/_client_rpc.py @@ -1,7 +1,8 @@ import asyncio import functools import logging -from collections.abc import Callable +from collections.abc import AsyncIterator, Callable +from contextlib import asynccontextmanager from dataclasses import dataclass from typing import Any @@ -156,3 +157,19 @@ async def unregister_handler(self, handler: Callable[..., Any]) -> None: raise RPCNotInitializedError await self._rpc.unregister(handler) + + +@asynccontextmanager +async def rabbitmq_rpc_client_context( + rpc_client_name: str, settings: RabbitSettings, **kwargs +) -> AsyncIterator[RabbitMQRPCClient]: + """ + Adapter to create and close a RabbitMQRPCClient using an async context manager. + """ + rpc_client = await RabbitMQRPCClient.create( + client_name=rpc_client_name, settings=settings, **kwargs + ) + try: + yield rpc_client + finally: + await rpc_client.close() diff --git a/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py b/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py new file mode 100644 index 000000000000..7b61ca2b9544 --- /dev/null +++ b/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py @@ -0,0 +1,142 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=unused-argument +# pylint: disable=unused-variable + +from collections.abc import AsyncIterator + +import pytest +import servicelib.fastapi.rabbitmq_lifespan +import servicelib.rabbitmq +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.rabbitmq_lifespan import ( + RabbitMQConfigurationError, + RabbitMQLifespanState, + rabbitmq_connectivity_lifespan, +) +from servicelib.rabbitmq import rabbitmq_rpc_client_context +from settings_library.application import BaseApplicationSettings +from settings_library.rabbit import RabbitSettings + + +@pytest.fixture +def mock_rabbitmq_connection(mocker: MockerFixture) -> MockType: + return mocker.patch.object( + servicelib.fastapi.rabbitmq_lifespan, + "wait_till_rabbitmq_responsive", + return_value=mocker.AsyncMock(), + ) + + +@pytest.fixture +def mock_rabbitmq_rpc_client_class(mocker: MockerFixture) -> MockType: + return mocker.patch.object( + servicelib.rabbitmq._client_rpc, + "RabbitMQRPCClient", + return_value=mocker.AsyncMock(), + ) + + +@pytest.fixture +def app_environment(monkeypatch: pytest.MonkeyPatch) -> EnvVarsDict: + return setenvs_from_dict( + monkeypatch, RabbitSettings.model_json_schema()["examples"][0] + ) + + +@pytest.fixture +def app_lifespan( + app_environment: EnvVarsDict, + mock_rabbitmq_connection: MockType, + mock_rabbitmq_rpc_client_class: MockType, +) -> LifespanManager: + assert app_environment + + class AppSettings(BaseApplicationSettings): + RABBITMQ: RabbitSettings = 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 RabbitMQLifespanState( + RABBIT_SETTINGS=app.state.settings.RABBITMQ, + ).model_dump() + + async def my_app_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: + + async with rabbitmq_rpc_client_context( + "rpc_server", app.state.settings.RABBITMQ + ) as rpc_server: + app.state.rpc_server = rpc_server + yield {} + + app_lifespan = LifespanManager() + app_lifespan.add(my_app_settings) + app_lifespan.add(rabbitmq_connectivity_lifespan) + app_lifespan.add(my_app_rpc_server) + + assert not mock_rabbitmq_connection.called + assert not mock_rabbitmq_rpc_client_class.called + + return app_lifespan + + +async def test_lifespan_rabbitmq_in_an_app( + is_pdb_enabled: bool, + app_environment: EnvVarsDict, + mock_rabbitmq_connection: MockType, + mock_rabbitmq_rpc_client_class: MockType, + app_lifespan: LifespanManager, +): + app = FastAPI(lifespan=app_lifespan) + + 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 RabbitMQ responsiveness was checked + mock_rabbitmq_connection.assert_called_once_with( + app.state.settings.RABBITMQ.dsn + ) + + # Verify that RabbitMQ settings are in the lifespan manager state + assert app.state.settings.RABBITMQ + + # No explicit shutdown logic for RabbitMQ in this case + assert mock_rabbitmq_rpc_client_class.called + + +async def test_lifespan_rabbitmq_with_invalid_settings( + is_pdb_enabled: bool, +): + async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: + yield {"RABBIT_SETTINGS": None} + + app_lifespan = LifespanManager() + app_lifespan.add(my_app_settings) + app_lifespan.add(rabbitmq_connectivity_lifespan) + + app = FastAPI(lifespan=app_lifespan) + + with pytest.raises(RabbitMQConfigurationError, match="Invalid RabbitMQ") as excinfo: + async with ASGILifespanManager( + app, + startup_timeout=None if is_pdb_enabled else 10, + shutdown_timeout=None if is_pdb_enabled else 10, + ): + ... + + exception = excinfo.value + assert isinstance(exception, RabbitMQConfigurationError) + assert exception.validation_error + assert exception.state["RABBIT_SETTINGS"] is None From cd98e87e3faecc971a6f8774dcce90b70f86d956 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 17 Apr 2025 00:35:47 +0200 Subject: [PATCH 6/9] =?UTF-8?q?=E2=9C=A8=20Refactor=20lifespan=20utility?= =?UTF-8?q?=20functions=20and=20enhance=20error=20handling=20in=20FastAPI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/lifespan_utils.py | 22 ++++++++++--- .../servicelib/fastapi/postgres_lifespan.py | 4 +-- .../servicelib/fastapi/rabbitmq_lifespan.py | 7 ++-- .../src/servicelib/fastapi/redis_lifespan.py | 4 +-- .../tests/fastapi/test_lifespan_utils.py | 15 ++++++--- .../tests/fastapi/test_rabbitmq_lifespan.py | 33 +++++++++++++++---- 6 files changed, 63 insertions(+), 22 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py index e8605e66ccdd..720fb6feb457 100644 --- a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py +++ b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py @@ -20,25 +20,37 @@ class LifespanAlreadyCalledError(LifespanError): msg_template = "The lifespan '{lifespan_name}' has already been called." +class LifespanExpectedCalledError(LifespanError): + msg_template = "The lifespan '{lifespan_name}' was not called. Ensure it is properly configured and invoked." + + _CALLED_LIFESPANS_KEY: Final[str] = "_CALLED_LIFESPANS" def is_lifespan_called(state: State, lifespan_name: str) -> bool: + assert not isinstance( # nosec + state, FastAPI + ), "TIP: lifespan func has (app, state) positional arguments" + called_lifespans = state.get(_CALLED_LIFESPANS_KEY, set()) return lifespan_name in called_lifespans -def record_lifespan_called_once(state: State, lifespan_name: str) -> State: +def mark_lifespace_called(state: State, lifespan_name: str) -> State: """Validates if a lifespan has already been called and records it in the state. Raises LifespanAlreadyCalledError if the lifespan has already been called. """ - assert not isinstance( # nosec - state, FastAPI - ), "TIP: lifespan func has (app, state) positional arguments" - if is_lifespan_called(state, lifespan_name): raise LifespanAlreadyCalledError(lifespan_name=lifespan_name) called_lifespans = state.get(_CALLED_LIFESPANS_KEY, set()) called_lifespans.add(lifespan_name) return {_CALLED_LIFESPANS_KEY: called_lifespans} + + +def ensure_lifespan_called(state: State, lifespan_name: str) -> None: + """Ensures that a lifespan has been called. + Raises LifespanNotCalledError if the lifespan has not been called. + """ + if not is_lifespan_called(state, lifespan_name): + raise LifespanExpectedCalledError(lifespan_name=lifespan_name) diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index 5d257725062d..695a1731a7b5 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -10,7 +10,7 @@ from sqlalchemy.ext.asyncio import AsyncEngine from ..db_asyncpg_utils import create_async_engine_and_database_ready -from .lifespan_utils import LifespanOnStartupError, record_lifespan_called_once +from .lifespan_utils import LifespanOnStartupError, mark_lifespace_called _logger = logging.getLogger(__name__) @@ -33,7 +33,7 @@ async def postgres_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[ with log_context(_logger, logging.INFO, f"{__name__}"): # Mark lifespan as called - called_state = record_lifespan_called_once(state, "postgres_database_lifespan") + called_state = mark_lifespace_called(state, "postgres_database_lifespan") settings = state[PostgresLifespanState.POSTGRES_SETTINGS] diff --git a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py index dfc27042de1a..41c46a4c2b57 100644 --- a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py @@ -8,7 +8,10 @@ from servicelib.rabbitmq import wait_till_rabbitmq_responsive from settings_library.rabbit import RabbitSettings -from .lifespan_utils import LifespanOnStartupError, record_lifespan_called_once +from .lifespan_utils import ( + LifespanOnStartupError, + mark_lifespace_called, +) _logger = logging.getLogger(__name__) @@ -33,7 +36,7 @@ async def rabbitmq_connectivity_lifespan( with log_context(_logger, logging.INFO, _lifespan_name): # Check if lifespan has already been called - called_state = record_lifespan_called_once(state, _lifespan_name) + called_state = mark_lifespace_called(state, _lifespan_name) # Validate input state try: diff --git a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py index 3424da575176..9b2aca8d75fe 100644 --- a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py @@ -10,7 +10,7 @@ from settings_library.redis import RedisDatabase, RedisSettings from ..redis import RedisClientSDK -from .lifespan_utils import LifespanOnStartupError, record_lifespan_called_once +from .lifespan_utils import LifespanOnStartupError, mark_lifespace_called _logger = logging.getLogger(__name__) @@ -34,7 +34,7 @@ async def redis_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[Sta with log_context(_logger, logging.INFO, f"{__name__}"): # Check if lifespan has already been called - called_state = record_lifespan_called_once(state, "redis_database_lifespan") + called_state = mark_lifespace_called(state, "redis_database_lifespan") # Validate input state try: diff --git a/packages/service-library/tests/fastapi/test_lifespan_utils.py b/packages/service-library/tests/fastapi/test_lifespan_utils.py index e247cf2a19b1..9f8baabf4304 100644 --- a/packages/service-library/tests/fastapi/test_lifespan_utils.py +++ b/packages/service-library/tests/fastapi/test_lifespan_utils.py @@ -17,9 +17,11 @@ from pytest_simcore.helpers.logging_tools import log_context from servicelib.fastapi.lifespan_utils import ( LifespanAlreadyCalledError, + LifespanExpectedCalledError, LifespanOnShutdownError, LifespanOnStartupError, - record_lifespan_called_once, + ensure_lifespan_called, + mark_lifespace_called, ) @@ -259,18 +261,21 @@ async def test_app_lifespan_with_error_on_shutdown( async def test_lifespan_called_more_than_once(is_pdb_enabled: bool): - state = {} - app_lifespan = LifespanManager() @app_lifespan.add async def _one(_, state: State) -> AsyncIterator[State]: - called_state = record_lifespan_called_once(state, "test_lifespan_one") + called_state = mark_lifespace_called(state, "test_lifespan_one") yield {"other": 0, **called_state} @app_lifespan.add async def _two(_, state: State) -> AsyncIterator[State]: - called_state = record_lifespan_called_once(state, "test_lifespan_two") + ensure_lifespan_called(state, "test_lifespan_one") + + with pytest.raises(LifespanExpectedCalledError): + ensure_lifespan_called(state, "test_lifespan_three") + + called_state = mark_lifespace_called(state, "test_lifespan_two") yield {"something": 0, **called_state} app_lifespan.add(_one) # added "by mistake" diff --git a/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py b/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py index 7b61ca2b9544..550aaeb5c815 100644 --- a/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py +++ b/packages/service-library/tests/fastapi/test_rabbitmq_lifespan.py @@ -37,11 +37,14 @@ def mock_rabbitmq_connection(mocker: MockerFixture) -> MockType: @pytest.fixture def mock_rabbitmq_rpc_client_class(mocker: MockerFixture) -> MockType: - return mocker.patch.object( - servicelib.rabbitmq._client_rpc, - "RabbitMQRPCClient", - return_value=mocker.AsyncMock(), + mock_rpc_client_instance = mocker.AsyncMock() + mocker.patch.object( + servicelib.rabbitmq._client_rpc.RabbitMQRPCClient, + "create", + return_value=mock_rpc_client_instance, ) + mock_rpc_client_instance.close = mocker.AsyncMock() + return mock_rpc_client_instance @pytest.fixture @@ -64,6 +67,7 @@ class AppSettings(BaseApplicationSettings): ..., json_schema_extra={"auto_default_from_env": True} ) + # setup settings async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: app.state.settings = AppSettings.create_from_envs() @@ -71,7 +75,9 @@ async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: RABBIT_SETTINGS=app.state.settings.RABBITMQ, ).model_dump() + # setup rpc-server using rabbitmq_rpc_client_context (yes, a "rpc_server" is built with an RabbitMQRpcClient) async def my_app_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: + assert "RABBIT_CONNECTIVITY_LIFESPAN_NAME" in state async with rabbitmq_rpc_client_context( "rpc_server", app.state.settings.RABBITMQ @@ -79,10 +85,22 @@ async def my_app_rpc_server(app: FastAPI, state: State) -> AsyncIterator[State]: app.state.rpc_server = rpc_server yield {} + # setup rpc-client using rabbitmq_rpc_client_context + async def my_app_rpc_client(app: FastAPI, state: State) -> AsyncIterator[State]: + + assert "RABBIT_CONNECTIVITY_LIFESPAN_NAME" in state + + async with rabbitmq_rpc_client_context( + "rpc_client", app.state.settings.RABBITMQ + ) as rpc_client: + app.state.rpc_client = rpc_client + yield {} + app_lifespan = LifespanManager() app_lifespan.add(my_app_settings) app_lifespan.add(rabbitmq_connectivity_lifespan) app_lifespan.add(my_app_rpc_server) + app_lifespan.add(my_app_rpc_client) assert not mock_rabbitmq_connection.called assert not mock_rabbitmq_rpc_client_class.called @@ -103,7 +121,8 @@ async def test_lifespan_rabbitmq_in_an_app( app, startup_timeout=None if is_pdb_enabled else 10, shutdown_timeout=None if is_pdb_enabled else 10, - ) as asgi_manager: + ): + # Verify that RabbitMQ responsiveness was checked mock_rabbitmq_connection.assert_called_once_with( app.state.settings.RABBITMQ.dsn @@ -111,9 +130,11 @@ async def test_lifespan_rabbitmq_in_an_app( # Verify that RabbitMQ settings are in the lifespan manager state assert app.state.settings.RABBITMQ + assert app.state.rpc_server + assert app.state.rpc_client # No explicit shutdown logic for RabbitMQ in this case - assert mock_rabbitmq_rpc_client_class.called + assert mock_rabbitmq_rpc_client_class.close.called async def test_lifespan_rabbitmq_with_invalid_settings( From b7a903926370fb944e76fbd051bc49f0473510e9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 17 Apr 2025 00:52:34 +0200 Subject: [PATCH 7/9] =?UTF-8?q?=E2=9C=A8=20Refactor=20lifespan=20managemen?= =?UTF-8?q?t=20by=20introducing=20a=20context=20manager=20for=20logging=20?= =?UTF-8?q?and=20state=20tracking=20in=20FastAPI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/lifespan_utils.py | 16 ++++++++++++++++ .../src/servicelib/fastapi/postgres_lifespan.py | 11 +++++------ .../src/servicelib/fastapi/rabbitmq_lifespan.py | 8 ++------ .../src/servicelib/fastapi/redis_lifespan.py | 14 ++++---------- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py index 720fb6feb457..d16ecd7c23f1 100644 --- a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py +++ b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py @@ -1,8 +1,11 @@ +import contextlib +from collections.abc import Iterator from typing import Final from common_library.errors_classes import OsparcErrorMixin from fastapi import FastAPI from fastapi_lifespan_manager import State +from servicelib.logging_utils import log_context class LifespanError(OsparcErrorMixin, RuntimeError): ... @@ -54,3 +57,16 @@ def ensure_lifespan_called(state: State, lifespan_name: str) -> None: """ if not is_lifespan_called(state, lifespan_name): raise LifespanExpectedCalledError(lifespan_name=lifespan_name) + + +@contextlib.contextmanager +def lifespan_context( + logger, level, lifespan_name: str, state: State +) -> Iterator[State]: + """Helper context manager to log lifespan event and mark lifespan as called.""" + + with log_context(logger, level, lifespan_name): + # Check if lifespan has already been called + called_state = mark_lifespace_called(state, lifespan_name) + + yield called_state diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index 695a1731a7b5..e54d6ed261bc 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -5,12 +5,12 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State -from servicelib.logging_utils import log_catch, log_context +from servicelib.logging_utils import log_catch from settings_library.postgres import PostgresSettings from sqlalchemy.ext.asyncio import AsyncEngine from ..db_asyncpg_utils import create_async_engine_and_database_ready -from .lifespan_utils import LifespanOnStartupError, mark_lifespace_called +from .lifespan_utils import LifespanOnStartupError, lifespan_context _logger = logging.getLogger(__name__) @@ -30,11 +30,10 @@ def create_postgres_database_input_state(settings: PostgresSettings) -> State: async def postgres_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[State]: - with log_context(_logger, logging.INFO, f"{__name__}"): - - # Mark lifespan as called - called_state = mark_lifespace_called(state, "postgres_database_lifespan") + _lifespan_name = f"{__name__}.{postgres_database_lifespan.__name__}" + with lifespan_context(_logger, logging.INFO, _lifespan_name, state) as called_state: + # Validate input state settings = state[PostgresLifespanState.POSTGRES_SETTINGS] if settings is None or not isinstance(settings, PostgresSettings): diff --git a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py index 41c46a4c2b57..06f3a9b2aefd 100644 --- a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py @@ -4,13 +4,12 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State from pydantic import BaseModel, ValidationError -from servicelib.logging_utils import log_context from servicelib.rabbitmq import wait_till_rabbitmq_responsive from settings_library.rabbit import RabbitSettings from .lifespan_utils import ( LifespanOnStartupError, - mark_lifespace_called, + lifespan_context, ) _logger = logging.getLogger(__name__) @@ -33,10 +32,7 @@ async def rabbitmq_connectivity_lifespan( """ _lifespan_name = f"{__name__}.{rabbitmq_connectivity_lifespan.__name__}" - with log_context(_logger, logging.INFO, _lifespan_name): - - # Check if lifespan has already been called - called_state = mark_lifespace_called(state, _lifespan_name) + with lifespan_context(_logger, logging.INFO, _lifespan_name, state) as called_state: # Validate input state try: diff --git a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py index 9b2aca8d75fe..ce2efe60ead8 100644 --- a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py @@ -5,12 +5,12 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State -from pydantic import BaseModel, ConfigDict, StringConstraints, ValidationError +from pydantic import BaseModel, StringConstraints, ValidationError from servicelib.logging_utils import log_catch, log_context from settings_library.redis import RedisDatabase, RedisSettings from ..redis import RedisClientSDK -from .lifespan_utils import LifespanOnStartupError, mark_lifespace_called +from .lifespan_utils import LifespanOnStartupError, lifespan_context _logger = logging.getLogger(__name__) @@ -24,17 +24,11 @@ class RedisLifespanState(BaseModel): REDIS_CLIENT_NAME: Annotated[str, StringConstraints(min_length=3, max_length=32)] REDIS_CLIENT_DB: RedisDatabase - model_config = ConfigDict( - extra="allow", - arbitrary_types_allowed=True, # RedisClientSDK has some arbitrary types and this class will never be serialized - ) - async def redis_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[State]: - with log_context(_logger, logging.INFO, f"{__name__}"): + _lifespan_name = f"{__name__}.{redis_database_lifespan.__name__}" - # Check if lifespan has already been called - called_state = mark_lifespace_called(state, "redis_database_lifespan") + with lifespan_context(_logger, logging.INFO, _lifespan_name, state) as called_state: # Validate input state try: From f7f3b249854d0494614f5952431af389d3cf71a5 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 17 Apr 2025 01:12:25 +0200 Subject: [PATCH 8/9] =?UTF-8?q?=E2=9C=A8=20Refactor=20import=20statements?= =?UTF-8?q?=20and=20update=20deprecated=20function=20usage=20in=20FastAPI?= =?UTF-8?q?=20lifespan=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/servicelib/fastapi/lifespan_utils.py | 3 ++- .../servicelib/fastapi/postgres_lifespan.py | 2 +- .../src/servicelib/fastapi/rabbitmq.py | 8 +++++++ .../servicelib/fastapi/rabbitmq_lifespan.py | 2 +- .../src/servicelib/fastapi/redis_lifespan.py | 21 ++++++++++--------- .../tests/fastapi/test_redis_lifespan.py | 6 +++--- 6 files changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py index d16ecd7c23f1..6f6d511ed632 100644 --- a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py +++ b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py @@ -5,7 +5,8 @@ from common_library.errors_classes import OsparcErrorMixin from fastapi import FastAPI from fastapi_lifespan_manager import State -from servicelib.logging_utils import log_context + +from ..logging_utils import log_context class LifespanError(OsparcErrorMixin, RuntimeError): ... diff --git a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py index e54d6ed261bc..319a7121896a 100644 --- a/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/postgres_lifespan.py @@ -5,11 +5,11 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State -from servicelib.logging_utils import log_catch from settings_library.postgres import PostgresSettings from sqlalchemy.ext.asyncio import AsyncEngine from ..db_asyncpg_utils import create_async_engine_and_database_ready +from ..logging_utils import log_catch from .lifespan_utils import LifespanOnStartupError, lifespan_context _logger = logging.getLogger(__name__) diff --git a/packages/service-library/src/servicelib/fastapi/rabbitmq.py b/packages/service-library/src/servicelib/fastapi/rabbitmq.py index 89f52099d564..4f41526c3abd 100644 --- a/packages/service-library/src/servicelib/fastapi/rabbitmq.py +++ b/packages/service-library/src/servicelib/fastapi/rabbitmq.py @@ -1,4 +1,5 @@ import logging +import warnings from fastapi import FastAPI from models_library.rabbitmq_messages import RabbitMessageBase @@ -55,6 +56,13 @@ def setup_rabbit( settings -- Rabbit settings or if None, the connection to rabbit is not done upon startup name -- name for the rmq client name """ + warnings.warn( + "The 'setup_rabbit' function is deprecated and will be removed in a future release. " + "Please use 'rabbitmq_lifespan' for managing RabbitMQ connections.", + DeprecationWarning, + stacklevel=2, + ) + app.state.rabbitmq_client = None # RabbitMQClient | None app.state.rabbitmq_client_name = name app.state.rabbitmq_settings = settings diff --git a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py index 06f3a9b2aefd..180dbad800e9 100644 --- a/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/rabbitmq_lifespan.py @@ -4,9 +4,9 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State from pydantic import BaseModel, ValidationError -from servicelib.rabbitmq import wait_till_rabbitmq_responsive from settings_library.rabbit import RabbitSettings +from ..rabbitmq import wait_till_rabbitmq_responsive from .lifespan_utils import ( LifespanOnStartupError, lifespan_context, diff --git a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py index ce2efe60ead8..b1ac98e9d6ca 100644 --- a/packages/service-library/src/servicelib/fastapi/redis_lifespan.py +++ b/packages/service-library/src/servicelib/fastapi/redis_lifespan.py @@ -6,9 +6,9 @@ from fastapi import FastAPI from fastapi_lifespan_manager import State from pydantic import BaseModel, StringConstraints, ValidationError -from servicelib.logging_utils import log_catch, log_context from settings_library.redis import RedisDatabase, RedisSettings +from ..logging_utils import log_catch, log_context from ..redis import RedisClientSDK from .lifespan_utils import LifespanOnStartupError, lifespan_context @@ -25,8 +25,8 @@ class RedisLifespanState(BaseModel): REDIS_CLIENT_DB: RedisDatabase -async def redis_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[State]: - _lifespan_name = f"{__name__}.{redis_database_lifespan.__name__}" +async def redis_client_sdk_lifespan(_: FastAPI, state: State) -> AsyncIterator[State]: + _lifespan_name = f"{__name__}.{redis_client_sdk_lifespan.__name__}" with lifespan_context(_logger, logging.INFO, _lifespan_name, state) as called_state: @@ -45,6 +45,8 @@ async def redis_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[Sta logging.INFO, f"Creating redis client with name={redis_state.REDIS_CLIENT_NAME}", ): + # NOTE: sdk integrats waiting until connection is ready + # and will raise an exception if it cannot connect redis_client = RedisClientSDK( redis_dsn_with_secrets, client_name=redis_state.REDIS_CLIENT_NAME, @@ -54,10 +56,9 @@ async def redis_database_lifespan(_: FastAPI, state: State) -> AsyncIterator[Sta yield {"REDIS_CLIENT_SDK": redis_client, **called_state} finally: # Teardown client - if redis_client: - with log_catch(_logger, reraise=False): - await asyncio.wait_for( - redis_client.shutdown(), - # NOTE: shutdown already has a _HEALTHCHECK_TASK_TIMEOUT_S of 10s - timeout=20, - ) + with log_catch(_logger, reraise=False): + await asyncio.wait_for( + redis_client.shutdown(), + # NOTE: shutdown already has a _HEALTHCHECK_TASK_TIMEOUT_S of 10s + timeout=20, + ) diff --git a/packages/service-library/tests/fastapi/test_redis_lifespan.py b/packages/service-library/tests/fastapi/test_redis_lifespan.py index 9095a271d215..8a30055c393e 100644 --- a/packages/service-library/tests/fastapi/test_redis_lifespan.py +++ b/packages/service-library/tests/fastapi/test_redis_lifespan.py @@ -19,7 +19,7 @@ from servicelib.fastapi.redis_lifespan import ( RedisConfigurationError, RedisLifespanState, - redis_database_lifespan, + redis_client_sdk_lifespan, ) from settings_library.application import BaseApplicationSettings from settings_library.redis import RedisDatabase, RedisSettings @@ -65,7 +65,7 @@ async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: app_lifespan = LifespanManager() app_lifespan.add(my_app_settings) - app_lifespan.add(redis_database_lifespan) + app_lifespan.add(redis_client_sdk_lifespan) assert not mock_redis_client_sdk.called @@ -112,7 +112,7 @@ async def my_app_settings(app: FastAPI) -> AsyncIterator[State]: app_lifespan = LifespanManager() app_lifespan.add(my_app_settings) - app_lifespan.add(redis_database_lifespan) + app_lifespan.add(redis_client_sdk_lifespan) app = FastAPI(lifespan=app_lifespan) From 33700536623ff32f2d3c08d4ae24a73caab84c8c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:50:43 +0200 Subject: [PATCH 9/9] @bisgaard-itis review: doc --- .../src/servicelib/fastapi/lifespan_utils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py index 6f6d511ed632..4ccf04109304 100644 --- a/packages/service-library/src/servicelib/fastapi/lifespan_utils.py +++ b/packages/service-library/src/servicelib/fastapi/lifespan_utils.py @@ -32,9 +32,13 @@ class LifespanExpectedCalledError(LifespanError): def is_lifespan_called(state: State, lifespan_name: str) -> bool: + # NOTE: This assert is meant to catch a common mistake: + # The `lifespan` function should accept up to two *optional* positional arguments: (app: FastAPI, state: State). + # Valid signatures include: `()`, `(app)`, `(app, state)`, or even `(_, state)`. + # It's easy to accidentally swap or misplace these arguments. assert not isinstance( # nosec state, FastAPI - ), "TIP: lifespan func has (app, state) positional arguments" + ), "Did you swap arguments? `lifespan(app, state)` expects (app: FastAPI, state: State)" called_lifespans = state.get(_CALLED_LIFESPANS_KEY, set()) return lifespan_name in called_lifespans