From 1630f5ba4fefacf2d0985b260ef35ae582c1e6a7 Mon Sep 17 00:00:00 2001 From: euri10 Date: Tue, 13 Feb 2024 14:27:04 +0100 Subject: [PATCH 01/29] Added store lifetime note --- docs/usage/stores.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/usage/stores.rst b/docs/usage/stores.rst index 52118f9332..fb580f75f7 100644 --- a/docs/usage/stores.rst +++ b/docs/usage/stores.rst @@ -259,3 +259,11 @@ with minimal boilerplate: Without any extra configuration, every call to ``app.stores.get`` with a unique name will return a namespace for this name only, while re-using the underlying Redis instance. + + +Store lifetime +++++++++++++++ + +Stores may not be automatically closed when the application is shut down. +This is the case in particular for the RedisStore if you are not using the class method :meth:`RedisStore.with_client <.redis.RedisStore.with_client>` and passing in your own Redis instance. +In this case you're responsible to close the Redis instance yourself. From c800ff723eee9e6bceecc5cca83986f9eecc9d3c Mon Sep 17 00:00:00 2001 From: euri10 Date: Tue, 13 Feb 2024 14:50:11 +0100 Subject: [PATCH 02/29] Added failing test from issue, will write a "simpler" one after hopefully --- tests/unit/test_stores.py | 60 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index da3761ff6c..382d55f1e1 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -9,16 +9,20 @@ from typing import TYPE_CHECKING, cast from unittest.mock import MagicMock, Mock, patch +import msgspec import pytest from _pytest.fixtures import FixtureRequest from pytest_mock import MockerFixture from time_machine import Coordinates +from litestar import Litestar, get from litestar.exceptions import ImproperlyConfiguredException +from litestar.middleware.session.server_side import ServerSideSessionConfig from litestar.stores.file import FileStore from litestar.stores.memory import MemoryStore from litestar.stores.redis import RedisStore from litestar.stores.registry import StoreRegistry +from litestar.testing import AsyncTestClient if TYPE_CHECKING: from redis.asyncio import Redis @@ -366,3 +370,59 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock await file_store.set("foo", "bar") mock_unlink.assert_called_once() assert Path(mock_unlink.call_args_list[0].args[0]).with_suffix("") == file_store.path.joinpath("foo") + + +# stores close +@get() +async def hget() -> int: + return 1 + + +class AppSettings(msgspec.Struct): + debug: bool + redis_url: str = "redis://localhost:6379" + + +def get_app(app_settings: AppSettings) -> Litestar: + # setting up stores + session_store = RedisStore.with_client(url=app_settings.redis_url) + app = Litestar( + route_handlers=[hget], + middleware=[ + ServerSideSessionConfig().middleware, + ], + stores={ + "sessions": session_store, + }, # comment this and 2nd test pass + debug=app_settings.debug, + ) + return app + + +@pytest.fixture(scope="session") +def anyio_backend() -> str: + return "asyncio" + + +@pytest.fixture(scope="session") +def app_settings_test(): + return AppSettings(debug=True) + + +@pytest.fixture(scope="session") +def app_test(app_settings_test: AppSettings): + app = get_app(app_settings_test) + yield app + + +@pytest.fixture # add scope="session" and 2nd test pass +async def client(app_test: Litestar): + async with AsyncTestClient(app=app_test) as c: + yield c + + +@pytest.mark.anyio +@pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) +async def test_param(client: AsyncTestClient, p1: int, p2: int): + response = await client.get("/") + assert response.status_code == 200 From 0523767679ac6a8c6ab433fc2745cdd3a61079dd Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 12:56:37 +0100 Subject: [PATCH 03/29] Make the RedisStore a context manager and see --- litestar/app.py | 3 +++ litestar/stores/redis.py | 26 +++++++++++++++++++++++--- tests/unit/test_stores.py | 7 +++---- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/litestar/app.py b/litestar/app.py index 48f3c33283..f43a0a7ea5 100644 --- a/litestar/app.py +++ b/litestar/app.py @@ -465,6 +465,9 @@ def __init__( self.stores: StoreRegistry = ( config.stores if isinstance(config.stores, StoreRegistry) else StoreRegistry(config.stores) ) + for _, store in self.stores._stores.items(): + if not store._should_close_store_manually: + self._lifespan_managers.append(store) @property @deprecated(version="2.6.0", kind="property", info="Use create_static_files router instead") diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index f0baa2538a..ce5e372ee7 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -1,7 +1,7 @@ from __future__ import annotations from datetime import timedelta -from typing import cast +from typing import TYPE_CHECKING, AsyncContextManager, cast from redis.asyncio import Redis from redis.asyncio.connection import ConnectionPool @@ -14,8 +14,11 @@ __all__ = ("RedisStore",) +if TYPE_CHECKING: + from types import TracebackType -class RedisStore(NamespacedStore): + +class RedisStore(NamespacedStore, AsyncContextManager): """Redis based, thread and process safe asynchronous key/value store.""" __slots__ = ("_redis",) @@ -30,6 +33,7 @@ def __init__(self, redis: Redis, namespace: str | None | EmptyType = Empty) -> N ``None``. This will make :meth:`.delete_all` unavailable. """ self._redis = redis + self._should_close_store_manually = True self.namespace: str | None = value_or_default(namespace, "LITESTAR") # script to get and renew a key in one atomic step @@ -64,6 +68,20 @@ def __init__(self, redis: Redis, namespace: str | None | EmptyType = Empty) -> N """ ) + async def _on_shutdown(self) -> None: + await self._redis.aclose() + + async def __aenter__(self) -> RedisStore: + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> None: + await self._on_shutdown() + @classmethod def with_client( cls, @@ -93,7 +111,9 @@ def with_client( username=username, password=password, ) - return cls(redis=Redis(connection_pool=pool), namespace=namespace) + i = cls(redis=Redis(connection_pool=pool), namespace=namespace) + i._should_close_store_manually = False + return i def with_namespace(self, namespace: str) -> RedisStore: """Return a new :class:`RedisStore` with a nested virtual key namespace. diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 382d55f1e1..704fcb2ea7 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -386,17 +386,16 @@ class AppSettings(msgspec.Struct): def get_app(app_settings: AppSettings) -> Litestar: # setting up stores session_store = RedisStore.with_client(url=app_settings.redis_url) - app = Litestar( + return Litestar( route_handlers=[hget], middleware=[ ServerSideSessionConfig().middleware, ], stores={ "sessions": session_store, - }, # comment this and 2nd test pass + }, debug=app_settings.debug, ) - return app @pytest.fixture(scope="session") @@ -415,7 +414,7 @@ def app_test(app_settings_test: AppSettings): yield app -@pytest.fixture # add scope="session" and 2nd test pass +@pytest.fixture async def client(app_test: Litestar): async with AsyncTestClient(app=app_test) as c: yield c From 8650cb60346f3cd6f16fa63d5ac9dc957017164a Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 18:45:59 +0100 Subject: [PATCH 04/29] Implement those good suggstions Note that close_connection_pool is required --- litestar/app.py | 13 ++++++------- litestar/stores/base.py | 15 +++++++++++++++ litestar/stores/redis.py | 31 +++++++++++++++++++------------ 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/litestar/app.py b/litestar/app.py index f43a0a7ea5..8a51d60f64 100644 --- a/litestar/app.py +++ b/litestar/app.py @@ -388,7 +388,13 @@ def __init__( self._openapi_schema: OpenAPI | None = None self._debug: bool = True + self.stores: StoreRegistry = ( + config.stores if isinstance(config.stores, StoreRegistry) else StoreRegistry(config.stores) + ) self._lifespan_managers = config.lifespan + for store in self.stores._stores.values(): + if store._automatic_store_lifetime: + self._lifespan_managers.append(store) self._server_lifespan_managers = [p.server_lifespan for p in config.plugins or [] if isinstance(p, CLIPlugin)] self.experimental_features = frozenset(config.experimental_features or []) self.get_logger: GetLogger = get_logger_placeholder @@ -462,13 +468,6 @@ def __init__( self.asgi_handler = self._create_asgi_handler() - self.stores: StoreRegistry = ( - config.stores if isinstance(config.stores, StoreRegistry) else StoreRegistry(config.stores) - ) - for _, store in self.stores._stores.items(): - if not store._should_close_store_manually: - self._lifespan_managers.append(store) - @property @deprecated(version="2.6.0", kind="property", info="Use create_static_files router instead") def static_files_config(self) -> list[StaticFilesConfig]: diff --git a/litestar/stores/base.py b/litestar/stores/base.py index 585a2cbf5b..10b23bd990 100644 --- a/litestar/stores/base.py +++ b/litestar/stores/base.py @@ -9,6 +9,8 @@ from msgspec.msgpack import encode as msgpack_encode if TYPE_CHECKING: + from types import TracebackType + from typing_extensions import Self @@ -76,6 +78,19 @@ async def expires_in(self, key: str) -> int | None: """ raise NotImplementedError + @abstractmethod + async def __aenter__(self) -> None: + ... + + @abstractmethod + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> None: + ... + class NamespacedStore(Store): """A subclass of :class:`Store`, offering hierarchical namespacing. diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index ce5e372ee7..9908bb7422 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -1,7 +1,7 @@ from __future__ import annotations from datetime import timedelta -from typing import TYPE_CHECKING, AsyncContextManager, cast +from typing import TYPE_CHECKING, cast from redis.asyncio import Redis from redis.asyncio.connection import ConnectionPool @@ -18,12 +18,14 @@ from types import TracebackType -class RedisStore(NamespacedStore, AsyncContextManager): +class RedisStore(NamespacedStore): """Redis based, thread and process safe asynchronous key/value store.""" __slots__ = ("_redis",) - def __init__(self, redis: Redis, namespace: str | None | EmptyType = Empty) -> None: + def __init__( + self, redis: Redis, namespace: str | None | EmptyType = Empty, automatic_store_lifetime: bool = False + ) -> None: """Initialize :class:`RedisStore` Args: @@ -31,10 +33,13 @@ def __init__(self, redis: Redis, namespace: str | None | EmptyType = Empty) -> N namespace: A key prefix to simulate a namespace in redis. If not given, defaults to ``LITESTAR``. Namespacing can be explicitly disabled by passing ``None``. This will make :meth:`.delete_all` unavailable. + automatic_store_lifetime: If ``True``, the store lifetime will be handled automatically, + works only if you're using the :meth:`.with_client` method. + By default it is False and in that case you will have to handle the store lifetime manually. """ self._redis = redis - self._should_close_store_manually = True self.namespace: str | None = value_or_default(namespace, "LITESTAR") + self._automatic_store_lifetime = automatic_store_lifetime # script to get and renew a key in one atomic step self._get_and_renew_script = self._redis.register_script( @@ -68,11 +73,11 @@ def __init__(self, redis: Redis, namespace: str | None | EmptyType = Empty) -> N """ ) - async def _on_shutdown(self) -> None: - await self._redis.aclose() + async def _shutdown(self) -> None: + await self._redis.aclose(close_connection_pool=True) - async def __aenter__(self) -> RedisStore: - return self + async def __aenter__(self) -> None: + ... async def __aexit__( self, @@ -80,7 +85,7 @@ async def __aexit__( exc_val: BaseException | None, exc_tb: TracebackType | None, ) -> None: - await self._on_shutdown() + await self._shutdown() @classmethod def with_client( @@ -92,6 +97,7 @@ def with_client( username: str | None = None, password: str | None = None, namespace: str | None | EmptyType = Empty, + automatic_store_lifetime: bool = True, ) -> RedisStore: """Initialize a :class:`RedisStore` instance with a new class:`redis.asyncio.Redis` instance. @@ -102,6 +108,7 @@ def with_client( username: Redis username to use password: Redis password to use namespace: Virtual key namespace to use + automatic_store_lifetime: If ``True``, the store lifetime will be handled automatically, it is the default with this method. """ pool = ConnectionPool.from_url( url=url, @@ -111,9 +118,9 @@ def with_client( username=username, password=password, ) - i = cls(redis=Redis(connection_pool=pool), namespace=namespace) - i._should_close_store_manually = False - return i + return cls( + redis=Redis(connection_pool=pool), namespace=namespace, automatic_store_lifetime=automatic_store_lifetime + ) def with_namespace(self, namespace: str) -> RedisStore: """Return a new :class:`RedisStore` with a nested virtual key namespace. From 6f566785c84699f4dc881dd70f416e2aa528e3bf Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:06:38 +0100 Subject: [PATCH 05/29] Better naming for the boolean that decides if we should handle stuff manually or not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Janek Nouvertné --- litestar/stores/redis.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index 9908bb7422..b0ed86d128 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -24,7 +24,7 @@ class RedisStore(NamespacedStore): __slots__ = ("_redis",) def __init__( - self, redis: Redis, namespace: str | None | EmptyType = Empty, automatic_store_lifetime: bool = False + self, redis: Redis, namespace: str | None | EmptyType = Empty, handle_client_shutdown: bool = False ) -> None: """Initialize :class:`RedisStore` @@ -33,9 +33,7 @@ def __init__( namespace: A key prefix to simulate a namespace in redis. If not given, defaults to ``LITESTAR``. Namespacing can be explicitly disabled by passing ``None``. This will make :meth:`.delete_all` unavailable. - automatic_store_lifetime: If ``True``, the store lifetime will be handled automatically, - works only if you're using the :meth:`.with_client` method. - By default it is False and in that case you will have to handle the store lifetime manually. + handle_client_shutdown: If ``True``, handle the shutdown of the `redis` instance automatically during the store's lifespan. Should be set to `True` unless the shutdown is handled externally """ self._redis = redis self.namespace: str | None = value_or_default(namespace, "LITESTAR") From bfe826247655ab65df75690b010fe9049aef9052 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:07:06 +0100 Subject: [PATCH 06/29] Removed the classmethod arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Janek Nouvertné --- litestar/stores/redis.py | 1 - 1 file changed, 1 deletion(-) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index b0ed86d128..8c31f413b2 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -95,7 +95,6 @@ def with_client( username: str | None = None, password: str | None = None, namespace: str | None | EmptyType = Empty, - automatic_store_lifetime: bool = True, ) -> RedisStore: """Initialize a :class:`RedisStore` instance with a new class:`redis.asyncio.Redis` instance. From 3cb8b3f2626eac15645c87af442c679b21192538 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:03:22 +0100 Subject: [PATCH 07/29] Make the async context manager not abstract --- litestar/stores/base.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/litestar/stores/base.py b/litestar/stores/base.py index 10b23bd990..6042a6057d 100644 --- a/litestar/stores/base.py +++ b/litestar/stores/base.py @@ -78,18 +78,16 @@ async def expires_in(self, key: str) -> int | None: """ raise NotImplementedError - @abstractmethod async def __aenter__(self) -> None: - ... + pass - @abstractmethod async def __aexit__( self, exc_type: type[BaseException] | None, exc_val: BaseException | None, exc_tb: TracebackType | None, ) -> None: - ... + pass class NamespacedStore(Store): From 3da05e5de28d536e1c3a4fda255be175e00f06c2 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:04:56 +0100 Subject: [PATCH 08/29] No need to implement __aenter__ anymore --- litestar/stores/redis.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index 8c31f413b2..dcc3fd6de1 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -74,9 +74,6 @@ def __init__( async def _shutdown(self) -> None: await self._redis.aclose(close_connection_pool=True) - async def __aenter__(self) -> None: - ... - async def __aexit__( self, exc_type: type[BaseException] | None, From f732783c3c170b87376bf4bba9d3d90150a36a42 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:10:58 +0100 Subject: [PATCH 09/29] Fix from previous changes and rebase --- litestar/app.py | 2 +- litestar/stores/redis.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/litestar/app.py b/litestar/app.py index 8a51d60f64..f24ec8007f 100644 --- a/litestar/app.py +++ b/litestar/app.py @@ -393,7 +393,7 @@ def __init__( ) self._lifespan_managers = config.lifespan for store in self.stores._stores.values(): - if store._automatic_store_lifetime: + if not store.handle_client_shutdown: self._lifespan_managers.append(store) self._server_lifespan_managers = [p.server_lifespan for p in config.plugins or [] if isinstance(p, CLIPlugin)] self.experimental_features = frozenset(config.experimental_features or []) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index dcc3fd6de1..b315f962e2 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -37,7 +37,7 @@ def __init__( """ self._redis = redis self.namespace: str | None = value_or_default(namespace, "LITESTAR") - self._automatic_store_lifetime = automatic_store_lifetime + self.handle_client_shutdown = handle_client_shutdown # script to get and renew a key in one atomic step self._get_and_renew_script = self._redis.register_script( @@ -113,7 +113,8 @@ def with_client( password=password, ) return cls( - redis=Redis(connection_pool=pool), namespace=namespace, automatic_store_lifetime=automatic_store_lifetime + redis=Redis(connection_pool=pool), + namespace=namespace, ) def with_namespace(self, namespace: str) -> RedisStore: From 18c4abab0bad2ad119c6478761592bc3a149cf19 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:14:48 +0100 Subject: [PATCH 10/29] Make the bool coherent with the docstring --- litestar/app.py | 2 +- litestar/stores/redis.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/litestar/app.py b/litestar/app.py index f24ec8007f..61123b9534 100644 --- a/litestar/app.py +++ b/litestar/app.py @@ -393,7 +393,7 @@ def __init__( ) self._lifespan_managers = config.lifespan for store in self.stores._stores.values(): - if not store.handle_client_shutdown: + if store.handle_client_shutdown: self._lifespan_managers.append(store) self._server_lifespan_managers = [p.server_lifespan for p in config.plugins or [] if isinstance(p, CLIPlugin)] self.experimental_features = frozenset(config.experimental_features or []) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index b315f962e2..0293b8a72a 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -115,6 +115,7 @@ def with_client( return cls( redis=Redis(connection_pool=pool), namespace=namespace, + handle_client_shutdown=True, ) def with_namespace(self, namespace: str) -> RedisStore: From 702d5ef6e1f8314c534f097481efcc6d9547d600 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 19:17:00 +0100 Subject: [PATCH 11/29] Avoid linter B027 empty-method-without-abstract-decorator on purpose --- litestar/stores/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/litestar/stores/base.py b/litestar/stores/base.py index 6042a6057d..34aa514fca 100644 --- a/litestar/stores/base.py +++ b/litestar/stores/base.py @@ -78,10 +78,10 @@ async def expires_in(self, key: str) -> int | None: """ raise NotImplementedError - async def __aenter__(self) -> None: + async def __aenter__(self) -> None: # noqa: B027 pass - async def __aexit__( + async def __aexit__( # noqa: B027 self, exc_type: type[BaseException] | None, exc_val: BaseException | None, From 545fe54ec54cd7b5c0ed1753adb5069a0f32ced8 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 20:49:50 +0100 Subject: [PATCH 12/29] Always add the store to lifespan_manager then let the store implementation decides to handle to close or not --- litestar/app.py | 3 +-- litestar/stores/redis.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/litestar/app.py b/litestar/app.py index 61123b9534..7839f3132e 100644 --- a/litestar/app.py +++ b/litestar/app.py @@ -393,8 +393,7 @@ def __init__( ) self._lifespan_managers = config.lifespan for store in self.stores._stores.values(): - if store.handle_client_shutdown: - self._lifespan_managers.append(store) + self._lifespan_managers.append(store) self._server_lifespan_managers = [p.server_lifespan for p in config.plugins or [] if isinstance(p, CLIPlugin)] self.experimental_features = frozenset(config.experimental_features or []) self.get_logger: GetLogger = get_logger_placeholder diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index 0293b8a72a..fc8a900e7f 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -72,7 +72,8 @@ def __init__( ) async def _shutdown(self) -> None: - await self._redis.aclose(close_connection_pool=True) + if self.handle_client_shutdown: + await self._redis.aclose(close_connection_pool=True) async def __aexit__( self, From 77ac9c0dcd4ad4aa8640719f570ca993791c57e1 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 21:07:21 +0100 Subject: [PATCH 13/29] Tamed mypy, aclose is definitely a Redis method, so not sure why it isn't recognized, we use types-redis --- litestar/stores/redis.py | 2 +- tests/unit/test_stores.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index fc8a900e7f..5db5ce2720 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -73,7 +73,7 @@ def __init__( async def _shutdown(self) -> None: if self.handle_client_shutdown: - await self._redis.aclose(close_connection_pool=True) + await self._redis.aclose(close_connection_pool=True) # type: ignore[attr-defined] async def __aexit__( self, diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 704fcb2ea7..36cd484f5a 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -6,7 +6,7 @@ import string from datetime import timedelta from pathlib import Path -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING, cast, Iterator from unittest.mock import MagicMock, Mock, patch import msgspec @@ -404,12 +404,12 @@ def anyio_backend() -> str: @pytest.fixture(scope="session") -def app_settings_test(): +def app_settings_test() -> AppSettings: return AppSettings(debug=True) @pytest.fixture(scope="session") -def app_test(app_settings_test: AppSettings): +def app_test(app_settings_test: AppSettings) -> Iterator[Litestar]: app = get_app(app_settings_test) yield app From 92d351691718d03a05ab26ea919f257ec70cdaa1 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 21:10:21 +0100 Subject: [PATCH 14/29] Ruff stuff --- tests/unit/test_stores.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 36cd484f5a..26a31bb90a 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -6,7 +6,7 @@ import string from datetime import timedelta from pathlib import Path -from typing import TYPE_CHECKING, cast, Iterator +from typing import TYPE_CHECKING, Iterator, cast from unittest.mock import MagicMock, Mock, patch import msgspec From 98ad6c817503e9b4d8b7d0236eec01a28bf72d12 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 21:17:40 +0100 Subject: [PATCH 15/29] Mypy on tests Added some small notes on the test since its setup is rather cumbersome --- tests/unit/test_stores.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 26a31bb90a..631b9d4658 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -6,7 +6,7 @@ import string from datetime import timedelta from pathlib import Path -from typing import TYPE_CHECKING, Iterator, cast +from typing import TYPE_CHECKING, AsyncIterator, Iterator, cast from unittest.mock import MagicMock, Mock, patch import msgspec @@ -372,19 +372,18 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock assert Path(mock_unlink.call_args_list[0].args[0]).with_suffix("") == file_store.path.joinpath("foo") -# stores close -@get() -async def hget() -> int: - return 1 - - +# test to check if a RedisStore created through with_client() is properly closed +# see https://github.com/litestar-org/litestar/issues/3083 class AppSettings(msgspec.Struct): debug: bool redis_url: str = "redis://localhost:6379" def get_app(app_settings: AppSettings) -> Litestar: - # setting up stores + @get() + async def hget() -> int: + return 1 + session_store = RedisStore.with_client(url=app_settings.redis_url) return Litestar( route_handlers=[hget], @@ -415,13 +414,14 @@ def app_test(app_settings_test: AppSettings) -> Iterator[Litestar]: @pytest.fixture -async def client(app_test: Litestar): +async def client(app_test: Litestar) -> AsyncIterator[AsyncTestClient]: async with AsyncTestClient(app=app_test) as c: yield c +# the test failed when using the RedisStore on the 2nd set of parameters @pytest.mark.anyio @pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) -async def test_param(client: AsyncTestClient, p1: int, p2: int): +async def test_param(client: AsyncTestClient, p1: int, p2: int) -> None: response = await client.get("/") assert response.status_code == 200 From ba6fcc237028979756a78b7dd130f3d6cb01c285 Mon Sep 17 00:00:00 2001 From: euri10 Date: Wed, 14 Feb 2024 21:23:31 +0100 Subject: [PATCH 16/29] Some flakiness? Trying to solve using the redis xdist --- tests/unit/test_stores.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 631b9d4658..6947cde826 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -420,6 +420,7 @@ async def client(app_test: Litestar) -> AsyncIterator[AsyncTestClient]: # the test failed when using the RedisStore on the 2nd set of parameters +@pytest.mark.xdist_group("redis") @pytest.mark.anyio @pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) async def test_param(client: AsyncTestClient, p1: int, p2: int) -> None: From 570ac9bb760711459badc690d9d7805d95c704a3 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 15 Feb 2024 08:59:10 +0100 Subject: [PATCH 17/29] Cleant the redis_client fixture making it flushall each time, and above all removing the Redis sync instance created there that was never closed --- tests/conftest.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4697a5eba9..858db7db5e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,7 +15,6 @@ import pytest from pytest_lazyfixture import lazy_fixture from redis.asyncio import Redis as AsyncRedis -from redis.client import Redis from time_machine import travel from litestar.logging import LoggingConfig @@ -306,13 +305,11 @@ def get_logger() -> GetLogger: @pytest.fixture() async def redis_client(docker_ip: str, redis_service: None) -> AsyncGenerator[AsyncRedis, None]: - # this is to get around some weirdness with pytest-asyncio and redis interaction - # on 3.8 and 3.9 - - Redis(host=docker_ip, port=6397).flushall() client: AsyncRedis = AsyncRedis(host=docker_ip, port=6397) yield client try: await client.aclose() # type: ignore[attr-defined] except RuntimeError: pass + finally: + await client.flushall() From e85b42826b7aed64c1de5e2cf3b046c6b80f77d9 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 15 Feb 2024 09:08:46 +0100 Subject: [PATCH 18/29] Made the introduced test redis "aware" --- tests/unit/test_stores.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 6947cde826..443a6fc7d3 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -423,6 +423,6 @@ async def client(app_test: Litestar) -> AsyncIterator[AsyncTestClient]: @pytest.mark.xdist_group("redis") @pytest.mark.anyio @pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) -async def test_param(client: AsyncTestClient, p1: int, p2: int) -> None: +async def test_param(client: AsyncTestClient, p1: int, p2: int, redis_service: None) -> None: response = await client.get("/") assert response.status_code == 200 From 7b4741b34831bb3c884a7e2fb5466d2e3cda2e28 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 15 Feb 2024 09:27:19 +0100 Subject: [PATCH 19/29] Just comment the introduced test to see if fxture change doesn't mess with existing suite --- tests/unit/test_stores.py | 104 +++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 443a6fc7d3..37b99154ae 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -374,55 +374,55 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock # test to check if a RedisStore created through with_client() is properly closed # see https://github.com/litestar-org/litestar/issues/3083 -class AppSettings(msgspec.Struct): - debug: bool - redis_url: str = "redis://localhost:6379" - - -def get_app(app_settings: AppSettings) -> Litestar: - @get() - async def hget() -> int: - return 1 - - session_store = RedisStore.with_client(url=app_settings.redis_url) - return Litestar( - route_handlers=[hget], - middleware=[ - ServerSideSessionConfig().middleware, - ], - stores={ - "sessions": session_store, - }, - debug=app_settings.debug, - ) - - -@pytest.fixture(scope="session") -def anyio_backend() -> str: - return "asyncio" - - -@pytest.fixture(scope="session") -def app_settings_test() -> AppSettings: - return AppSettings(debug=True) - - -@pytest.fixture(scope="session") -def app_test(app_settings_test: AppSettings) -> Iterator[Litestar]: - app = get_app(app_settings_test) - yield app - - -@pytest.fixture -async def client(app_test: Litestar) -> AsyncIterator[AsyncTestClient]: - async with AsyncTestClient(app=app_test) as c: - yield c - - -# the test failed when using the RedisStore on the 2nd set of parameters -@pytest.mark.xdist_group("redis") -@pytest.mark.anyio -@pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) -async def test_param(client: AsyncTestClient, p1: int, p2: int, redis_service: None) -> None: - response = await client.get("/") - assert response.status_code == 200 +# class AppSettings(msgspec.Struct): +# debug: bool +# redis_url: str = "redis://localhost:6379" +# +# +# def get_app(app_settings: AppSettings) -> Litestar: +# @get() +# async def hget() -> int: +# return 1 +# +# session_store = RedisStore.with_client(url=app_settings.redis_url) +# return Litestar( +# route_handlers=[hget], +# middleware=[ +# ServerSideSessionConfig().middleware, +# ], +# stores={ +# "sessions": session_store, +# }, +# debug=app_settings.debug, +# ) +# +# +# @pytest.fixture(scope="session") +# def anyio_backend() -> str: +# return "asyncio" +# +# +# @pytest.fixture(scope="session") +# def app_settings_test() -> AppSettings: +# return AppSettings(debug=True) +# +# +# @pytest.fixture(scope="session") +# def app_test(app_settings_test: AppSettings) -> Iterator[Litestar]: +# app = get_app(app_settings_test) +# yield app +# +# +# @pytest.fixture +# async def client(app_test: Litestar) -> AsyncIterator[AsyncTestClient]: +# async with AsyncTestClient(app=app_test) as c: +# yield c +# +# +# # the test failed when using the RedisStore on the 2nd set of parameters +# @pytest.mark.xdist_group("redis") +# @pytest.mark.anyio +# @pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) +# async def test_param(client: AsyncTestClient, p1: int, p2: int) -> None: +# response = await client.get("/") +# assert response.status_code == 200 From 45ee20e4e561f475cd351ad5476c9e5f8767aadf Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 15 Feb 2024 10:33:01 +0100 Subject: [PATCH 20/29] Do we need really a test, or should it uses a mock like all times with_client is used --- tests/unit/test_stores.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 37b99154ae..5e1bc74f02 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -6,23 +6,19 @@ import string from datetime import timedelta from pathlib import Path -from typing import TYPE_CHECKING, AsyncIterator, Iterator, cast +from typing import TYPE_CHECKING, cast from unittest.mock import MagicMock, Mock, patch -import msgspec import pytest from _pytest.fixtures import FixtureRequest from pytest_mock import MockerFixture from time_machine import Coordinates -from litestar import Litestar, get from litestar.exceptions import ImproperlyConfiguredException -from litestar.middleware.session.server_side import ServerSideSessionConfig from litestar.stores.file import FileStore from litestar.stores.memory import MemoryStore from litestar.stores.redis import RedisStore from litestar.stores.registry import StoreRegistry -from litestar.testing import AsyncTestClient if TYPE_CHECKING: from redis.asyncio import Redis @@ -376,7 +372,7 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock # see https://github.com/litestar-org/litestar/issues/3083 # class AppSettings(msgspec.Struct): # debug: bool -# redis_url: str = "redis://localhost:6379" +# redis_url: str = "redis://127.0.0.1:6379" # # # def get_app(app_settings: AppSettings) -> Litestar: @@ -398,11 +394,6 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock # # # @pytest.fixture(scope="session") -# def anyio_backend() -> str: -# return "asyncio" -# -# -# @pytest.fixture(scope="session") # def app_settings_test() -> AppSettings: # return AppSettings(debug=True) # @@ -420,8 +411,6 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock # # # # the test failed when using the RedisStore on the 2nd set of parameters -# @pytest.mark.xdist_group("redis") -# @pytest.mark.anyio # @pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) # async def test_param(client: AsyncTestClient, p1: int, p2: int) -> None: # response = await client.get("/") From 40f2253ab8d3aea4aea0809761e8e661b42895a0 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 15 Feb 2024 10:38:22 +0100 Subject: [PATCH 21/29] Do we need really a test, or should it uses a mock like all times with_client is used --- tests/unit/test_stores.py | 49 --------------------------------------- 1 file changed, 49 deletions(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 5e1bc74f02..da3761ff6c 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -366,52 +366,3 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock await file_store.set("foo", "bar") mock_unlink.assert_called_once() assert Path(mock_unlink.call_args_list[0].args[0]).with_suffix("") == file_store.path.joinpath("foo") - - -# test to check if a RedisStore created through with_client() is properly closed -# see https://github.com/litestar-org/litestar/issues/3083 -# class AppSettings(msgspec.Struct): -# debug: bool -# redis_url: str = "redis://127.0.0.1:6379" -# -# -# def get_app(app_settings: AppSettings) -> Litestar: -# @get() -# async def hget() -> int: -# return 1 -# -# session_store = RedisStore.with_client(url=app_settings.redis_url) -# return Litestar( -# route_handlers=[hget], -# middleware=[ -# ServerSideSessionConfig().middleware, -# ], -# stores={ -# "sessions": session_store, -# }, -# debug=app_settings.debug, -# ) -# -# -# @pytest.fixture(scope="session") -# def app_settings_test() -> AppSettings: -# return AppSettings(debug=True) -# -# -# @pytest.fixture(scope="session") -# def app_test(app_settings_test: AppSettings) -> Iterator[Litestar]: -# app = get_app(app_settings_test) -# yield app -# -# -# @pytest.fixture -# async def client(app_test: Litestar) -> AsyncIterator[AsyncTestClient]: -# async with AsyncTestClient(app=app_test) as c: -# yield c -# -# -# # the test failed when using the RedisStore on the 2nd set of parameters -# @pytest.mark.parametrize("p1, p2", [(1, 2), (3, 4)]) -# async def test_param(client: AsyncTestClient, p1: int, p2: int) -> None: -# response = await client.get("/") -# assert response.status_code == 200 From f7ae11635f528c58e2e31758e53b392f4e7688e8 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 15 Feb 2024 15:59:51 +0100 Subject: [PATCH 22/29] Removed sync redis instance from fixture --- tests/conftest.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4697a5eba9..fc6e0a1cd0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,7 +15,6 @@ import pytest from pytest_lazyfixture import lazy_fixture from redis.asyncio import Redis as AsyncRedis -from redis.client import Redis from time_machine import travel from litestar.logging import LoggingConfig @@ -306,13 +305,6 @@ def get_logger() -> GetLogger: @pytest.fixture() async def redis_client(docker_ip: str, redis_service: None) -> AsyncGenerator[AsyncRedis, None]: - # this is to get around some weirdness with pytest-asyncio and redis interaction - # on 3.8 and 3.9 - - Redis(host=docker_ip, port=6397).flushall() - client: AsyncRedis = AsyncRedis(host=docker_ip, port=6397) - yield client - try: - await client.aclose() # type: ignore[attr-defined] - except RuntimeError: - pass + async with AsyncRedis(host=docker_ip, port=6397) as client: + yield client + await client.flushall() From e2bc0c8bd8bdd9b5fbb64fff22cf458975422da7 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 22 Feb 2024 15:13:31 +0100 Subject: [PATCH 23/29] You cannot yield the redis client weirdly enough... --- tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index fc6e0a1cd0..873b13f082 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,7 @@ from datetime import datetime from os import urandom from pathlib import Path -from typing import TYPE_CHECKING, Any, AsyncGenerator, Callable, Generator, TypeVar, Union, cast +from typing import TYPE_CHECKING, Any, Callable, Generator, TypeVar, Union, cast from unittest.mock import AsyncMock, MagicMock import pytest @@ -304,7 +304,7 @@ def get_logger() -> GetLogger: @pytest.fixture() -async def redis_client(docker_ip: str, redis_service: None) -> AsyncGenerator[AsyncRedis, None]: +async def redis_client(docker_ip: str, redis_service: None) -> AsyncRedis: async with AsyncRedis(host=docker_ip, port=6397) as client: - yield client await client.flushall() + return client From 58272d4d23c457ca0d22499ec553389719176a46 Mon Sep 17 00:00:00 2001 From: euri10 Date: Thu, 22 Feb 2024 15:17:02 +0100 Subject: [PATCH 24/29] Use async context manager in docker stuff as well --- tests/docker_service_fixtures.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/docker_service_fixtures.py b/tests/docker_service_fixtures.py index 6efca36979..0efa26fd61 100644 --- a/tests/docker_service_fixtures.py +++ b/tests/docker_service_fixtures.py @@ -113,13 +113,11 @@ def docker_ip(docker_services: DockerServiceRegistry) -> str: async def redis_responsive(host: str) -> bool: - client: AsyncRedis = AsyncRedis(host=host, port=6397) - try: - return await client.ping() - except (ConnectionError, RedisConnectionError): - return False - finally: - await client.aclose() + async with AsyncRedis(host=host, port=6397) as client: + try: + return await client.ping() + except (ConnectionError, RedisConnectionError): + return False @pytest.fixture() From 3fc2cf26e2dca7fb27adc4498a16d4704fb6d54e Mon Sep 17 00:00:00 2001 From: euri10 Date: Sun, 25 Feb 2024 08:40:18 +0100 Subject: [PATCH 25/29] We need to explicitely pass this otherwise using with_namespace after with_client will lose the automatic handling --- litestar/stores/redis.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index 5db5ce2720..a34b5f225b 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -124,7 +124,11 @@ def with_namespace(self, namespace: str) -> RedisStore: The current instances namespace will serve as a prefix for the namespace, so it can be considered the parent namespace. """ - return type(self)(redis=self._redis, namespace=f"{self.namespace}_{namespace}" if self.namespace else namespace) + return type(self)( + redis=self._redis, + namespace=f"{self.namespace}_{namespace}" if self.namespace else namespace, + handle_client_shutdown=self.handle_client_shutdown, + ) def _make_key(self, key: str) -> str: prefix = f"{self.namespace}:" if self.namespace else "" From 99a814d176e7614a8cb4bcf4441681dfce03598b Mon Sep 17 00:00:00 2001 From: euri10 Date: Mon, 26 Feb 2024 08:26:00 +0100 Subject: [PATCH 26/29] Let's keep the fixtures as-is --- tests/conftest.py | 18 +++++++++++++----- tests/docker_service_fixtures.py | 12 +++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 873b13f082..4697a5eba9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,12 +9,13 @@ from datetime import datetime from os import urandom from pathlib import Path -from typing import TYPE_CHECKING, Any, Callable, Generator, TypeVar, Union, cast +from typing import TYPE_CHECKING, Any, AsyncGenerator, Callable, Generator, TypeVar, Union, cast from unittest.mock import AsyncMock, MagicMock import pytest from pytest_lazyfixture import lazy_fixture from redis.asyncio import Redis as AsyncRedis +from redis.client import Redis from time_machine import travel from litestar.logging import LoggingConfig @@ -304,7 +305,14 @@ def get_logger() -> GetLogger: @pytest.fixture() -async def redis_client(docker_ip: str, redis_service: None) -> AsyncRedis: - async with AsyncRedis(host=docker_ip, port=6397) as client: - await client.flushall() - return client +async def redis_client(docker_ip: str, redis_service: None) -> AsyncGenerator[AsyncRedis, None]: + # this is to get around some weirdness with pytest-asyncio and redis interaction + # on 3.8 and 3.9 + + Redis(host=docker_ip, port=6397).flushall() + client: AsyncRedis = AsyncRedis(host=docker_ip, port=6397) + yield client + try: + await client.aclose() # type: ignore[attr-defined] + except RuntimeError: + pass diff --git a/tests/docker_service_fixtures.py b/tests/docker_service_fixtures.py index 0efa26fd61..6efca36979 100644 --- a/tests/docker_service_fixtures.py +++ b/tests/docker_service_fixtures.py @@ -113,11 +113,13 @@ def docker_ip(docker_services: DockerServiceRegistry) -> str: async def redis_responsive(host: str) -> bool: - async with AsyncRedis(host=host, port=6397) as client: - try: - return await client.ping() - except (ConnectionError, RedisConnectionError): - return False + client: AsyncRedis = AsyncRedis(host=host, port=6397) + try: + return await client.ping() + except (ConnectionError, RedisConnectionError): + return False + finally: + await client.aclose() @pytest.fixture() From fd2b54bef3b2ec629fe7c30516aca9a4b3af5e01 Mon Sep 17 00:00:00 2001 From: euri10 Date: Mon, 26 Feb 2024 10:00:33 +0100 Subject: [PATCH 27/29] Add a test for with_client _shutdown --- tests/unit/test_stores.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index da3761ff6c..93ae2cf659 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -366,3 +366,16 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock await file_store.set("foo", "bar") mock_unlink.assert_called_once() assert Path(mock_unlink.call_args_list[0].args[0]).with_suffix("") == file_store.path.joinpath("foo") + + +async def test_redis_store_with_client_shutdown(redis_service: None) -> None: + redis_store = RedisStore.with_client(url="redis://localhost:6397") + assert await redis_store._redis.ping() + # remove the private shutdown and the assert below fails + # the check on connection is a mimic of https://github.com/redis/redis-py/blob/d529c2ad8d2cf4dcfb41bfd93ea68cfefd81aa66/tests/test_asyncio/test_connection_pool.py#L35-L39 + await redis_store._shutdown() + assert not any( + x.is_connected + for x in redis_store._redis.connection_pool._available_connections + + list(redis_store._redis.connection_pool._in_use_connections) + ) From 3651fe54f5dbb3b6e27d3b2c1bebc4748cbf574b Mon Sep 17 00:00:00 2001 From: euri10 Date: Mon, 26 Feb 2024 10:12:33 +0100 Subject: [PATCH 28/29] Remove the redis_service fixture from the test, note that it won't run on its own now but most likely will please the CI --- tests/unit/test_stores.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_stores.py b/tests/unit/test_stores.py index 93ae2cf659..c8178996bc 100644 --- a/tests/unit/test_stores.py +++ b/tests/unit/test_stores.py @@ -368,7 +368,7 @@ async def test_file_store_handle_rename_fail(file_store: FileStore, mocker: Mock assert Path(mock_unlink.call_args_list[0].args[0]).with_suffix("") == file_store.path.joinpath("foo") -async def test_redis_store_with_client_shutdown(redis_service: None) -> None: +async def test_redis_store_with_client_shutdown() -> None: redis_store = RedisStore.with_client(url="redis://localhost:6397") assert await redis_store._redis.ping() # remove the private shutdown and the assert below fails From 2c91e099a8e25c796b04c8d511ae433a9aee74d5 Mon Sep 17 00:00:00 2001 From: euri10 Date: Mon, 26 Feb 2024 10:37:49 +0100 Subject: [PATCH 29/29] Update litestar/stores/redis.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Janek Nouvertné --- litestar/stores/redis.py | 1 - 1 file changed, 1 deletion(-) diff --git a/litestar/stores/redis.py b/litestar/stores/redis.py index a34b5f225b..6697962fab 100644 --- a/litestar/stores/redis.py +++ b/litestar/stores/redis.py @@ -103,7 +103,6 @@ def with_client( username: Redis username to use password: Redis password to use namespace: Virtual key namespace to use - automatic_store_lifetime: If ``True``, the store lifetime will be handled automatically, it is the default with this method. """ pool = ConnectionPool.from_url( url=url,