ongoing

Author: sanderegg

packages/service-library/src/servicelib/redis/_client.py

@@ -114,10 +114,12 @@ def is_healthy(self) -> bool:
         return self._is_healthy
 
     def create_lock(
-        self, lock_name: str, *, ttl: datetime.timedelta = DEFAULT_LOCK_TTL
+        self, lock_name: str, *, ttl: datetime.timedelta | None = DEFAULT_LOCK_TTL
     ) -> Lock:
         return self._client.lock(
-            name=lock_name, timeout=ttl.total_seconds(), blocking=False
+            name=lock_name,
+            timeout=ttl.total_seconds() if ttl is not None else None,
+            blocking=False,
         )
 
     async def lock_value(self, lock_name: str) -> str | None:

packages/service-library/tests/redis/test_client.py

@@ -8,20 +8,13 @@
 import datetime
 from collections.abc import AsyncIterator, Callable
 from contextlib import AbstractAsyncContextManager
-from typing import Final
 
 import pytest
 from faker import Faker
 from pytest_mock import MockerFixture
 from redis.exceptions import LockError, LockNotOwnedError
-from servicelib.redis import (
-    CouldNotAcquireLockError,
-    RedisClientSDK,
-    RedisClientsManager,
-    RedisManagerDBConfig,
-)
+from servicelib.redis import RedisClientSDK, RedisClientsManager, RedisManagerDBConfig
 from servicelib.redis import _constants as redis_constants
-from servicelib.utils import limited_gather
 from settings_library.redis import RedisDatabase, RedisSettings
 from tenacity import (
     AsyncRetrying,
@@ -39,11 +32,6 @@
 ]
 
 
-async def _is_locked(redis_client_sdk: RedisClientSDK, lock_name: str) -> bool:
-    lock = redis_client_sdk.redis.lock(lock_name)
-    return await lock.locked()
-
-
 @pytest.fixture
 async def redis_client_sdk(
     get_redis_client_sdk: Callable[
@@ -66,34 +54,28 @@ def with_short_default_redis_lock_ttl(mocker: MockerFixture) -> None:
     )
 
 
-async def test_redis_key_encode_decode(redis_client_sdk: RedisClientSDK, faker: Faker):
-    key = faker.pystr()
-    value = faker.pystr()
-    await redis_client_sdk.redis.set(key, value)
-    val = await redis_client_sdk.redis.get(key)
-    assert val == value
-    await redis_client_sdk.redis.delete(key)
+@pytest.fixture
+def lock_name(faker: Faker) -> str:
+    return faker.pystr()
 
 
-async def test_redis_lock_acquisition(redis_client_sdk: RedisClientSDK, faker: Faker):
-    lock_name = faker.pystr()
-    lock = redis_client_sdk.redis.lock(lock_name)
+async def test_redis_lock_no_ttl(redis_client_sdk: RedisClientSDK, lock_name: str):
+    lock = redis_client_sdk.create_lock(lock_name, ttl=None)
     assert await lock.locked() is False
 
-    # Try to acquire the lock:
     lock_acquired = await lock.acquire(blocking=False)
     assert lock_acquired is True
     assert await lock.locked() is True
     assert await lock.owned() is True
     with pytest.raises(LockError):
-        # a lock with no timeout cannot be reacquired
+        # a lock with no ttl cannot be reacquired
         await lock.reacquire()
     with pytest.raises(LockError):
-        # a lock with no timeout cannot be extended
+        # a lock with no ttl cannot be extended
         await lock.extend(2)
 
     # try to acquire the lock a second time
-    same_lock = redis_client_sdk.redis.lock(lock_name)
+    same_lock = redis_client_sdk.create_lock(lock_name, ttl=None)
     assert await same_lock.locked() is True
     assert await same_lock.owned() is False
     assert await same_lock.acquire(blocking=False) is False
@@ -104,11 +86,10 @@ async def test_redis_lock_acquisition(redis_client_sdk: RedisClientSDK, faker: F
     assert not await lock.owned()
 
 
-async def test_redis_lock_context_manager(
-    redis_client_sdk: RedisClientSDK, faker: Faker
+async def test_redis_lock_context_manager_no_ttl(
+    redis_client_sdk: RedisClientSDK, lock_name: str
 ):
-    lock_name = faker.pystr()
-    lock = redis_client_sdk.redis.lock(lock_name)
+    lock = redis_client_sdk.create_lock(lock_name, ttl=None)
     assert not await lock.locked()
 
     async with lock:
@@ -123,7 +104,7 @@ async def test_redis_lock_context_manager(
             await lock.extend(2)
 
         # try to acquire the lock a second time
-        same_lock = redis_client_sdk.redis.lock(lock_name, blocking_timeout=1)
+        same_lock = redis_client_sdk.create_lock(lock_name, ttl=None)
         assert await same_lock.locked()
         assert not await same_lock.owned()
         assert await same_lock.acquire() is False
@@ -134,11 +115,9 @@ async def test_redis_lock_context_manager(
 
 
 async def test_redis_lock_with_ttl(
-    redis_client_sdk: RedisClientSDK, faker: Faker, redis_lock_ttl: datetime.timedelta
+    redis_client_sdk: RedisClientSDK, lock_name: str, redis_lock_ttl: datetime.timedelta
 ):
-    ttl_lock = redis_client_sdk.redis.lock(
-        faker.pystr(), timeout=redis_lock_ttl.total_seconds()
-    )
+    ttl_lock = redis_client_sdk.create_lock(lock_name, ttl=redis_lock_ttl)
     assert not await ttl_lock.locked()
 
     with pytest.raises(LockNotOwnedError):  # noqa: PT012
@@ -150,104 +129,104 @@ async def test_redis_lock_with_ttl(
             assert not await ttl_lock.locked()
 
 
-async def test_lock_context_with_already_locked_lock_raises(
-    redis_client_sdk: RedisClientSDK, faker: Faker
-):
-    lock_name = faker.pystr()
-    assert await _is_locked(redis_client_sdk, lock_name) is False
-    async with redis_client_sdk.lock_context(lock_name) as lock:
-        assert await _is_locked(redis_client_sdk, lock_name) is True
-
-        assert isinstance(lock.name, str)
-
-        # case where gives up immediately to acquire lock without waiting
-        with pytest.raises(CouldNotAcquireLockError):
-            async with redis_client_sdk.lock_context(lock.name, blocking=False):
-                ...
-
-        # case when lock waits up to blocking_timeout_s before giving up on
-        # lock acquisition
-        with pytest.raises(CouldNotAcquireLockError):
-            async with redis_client_sdk.lock_context(
-                lock.name, blocking=True, blocking_timeout_s=0.1
-            ):
-                ...
-
-        assert await lock.locked() is True
-    assert await _is_locked(redis_client_sdk, lock_name) is False
-
-
-async def test_lock_context_with_data(redis_client_sdk: RedisClientSDK, faker: Faker):
-    lock_data = faker.text()
-    lock_name = faker.pystr()
-    assert await _is_locked(redis_client_sdk, lock_name) is False
-    assert await redis_client_sdk.lock_value(lock_name) is None
-    async with redis_client_sdk.lock_context(lock_name, lock_value=lock_data):
-        assert await _is_locked(redis_client_sdk, lock_name) is True
-        assert await redis_client_sdk.lock_value(lock_name) == lock_data
-    assert await _is_locked(redis_client_sdk, lock_name) is False
-    assert await redis_client_sdk.lock_value(lock_name) is None
-
-
-async def test_lock_context_released_after_error(
-    redis_client_sdk: RedisClientSDK, faker: Faker
-):
-    lock_name = faker.pystr()
-
-    assert await redis_client_sdk.lock_value(lock_name) is None
-
-    with pytest.raises(RuntimeError):  # noqa: PT012
-        async with redis_client_sdk.lock_context(lock_name):
-            assert await redis_client_sdk.redis.get(lock_name) is not None
-            msg = "Expected error"
-            raise RuntimeError(msg)
-
-    assert await redis_client_sdk.lock_value(lock_name) is None
-
-
-async def test_lock_acquired_in_parallel_to_update_same_resource(
-    with_short_default_redis_lock_ttl: None,
-    get_redis_client_sdk: Callable[
-        [RedisDatabase], AbstractAsyncContextManager[RedisClientSDK]
-    ],
-    faker: Faker,
-):
-    INCREASE_OPERATIONS: Final[int] = 250
-    INCREASE_BY: Final[int] = 10
-
-    class RaceConditionCounter:
-        def __init__(self):
-            self.value: int = 0
-
-        async def race_condition_increase(self, by: int) -> None:
-            current_value = self.value
-            current_value += by
-            # most likely situation which creates issues
-            await asyncio.sleep(redis_constants.DEFAULT_LOCK_TTL.total_seconds() / 2)
-            self.value = current_value
-
-    counter = RaceConditionCounter()
-    lock_name: str = faker.pystr()
-    # ensures it does nto time out before acquiring the lock
-    time_for_all_inc_counter_calls_to_finish_s: float = (
-        redis_constants.DEFAULT_LOCK_TTL.total_seconds() * INCREASE_OPERATIONS * 10
-    )
-
-    async def _inc_counter() -> None:
-        async with get_redis_client_sdk(  # noqa: SIM117
-            RedisDatabase.RESOURCES
-        ) as redis_client_sdk:
-            async with redis_client_sdk.lock_context(
-                lock_key=lock_name,
-                blocking=True,
-                blocking_timeout_s=time_for_all_inc_counter_calls_to_finish_s,
-            ):
-                await counter.race_condition_increase(INCREASE_BY)
-
-    await limited_gather(
-        *(_inc_counter() for _ in range(INCREASE_OPERATIONS)), limit=15
-    )
-    assert counter.value == INCREASE_BY * INCREASE_OPERATIONS
+# async def test_lock_context_with_already_locked_lock_raises(
+#     redis_client_sdk: RedisClientSDK, faker: Faker
+# ):
+#     lock_name = faker.pystr()
+#     assert await _is_locked(redis_client_sdk, lock_name) is False
+#     async with redis_client_sdk.lock_context(lock_name) as lock:
+#         assert await _is_locked(redis_client_sdk, lock_name) is True
+
+#         assert isinstance(lock.name, str)
+
+#         # case where gives up immediately to acquire lock without waiting
+#         with pytest.raises(CouldNotAcquireLockError):
+#             async with redis_client_sdk.lock_context(lock.name, blocking=False):
+#                 ...
+
+#         # case when lock waits up to blocking_timeout_s before giving up on
+#         # lock acquisition
+#         with pytest.raises(CouldNotAcquireLockError):
+#             async with redis_client_sdk.lock_context(
+#                 lock.name, blocking=True, blocking_timeout_s=0.1
+#             ):
+#                 ...
+
+#         assert await lock.locked() is True
+#     assert await _is_locked(redis_client_sdk, lock_name) is False
+
+
+# async def test_lock_context_with_data(redis_client_sdk: RedisClientSDK, faker: Faker):
+#     lock_data = faker.text()
+#     lock_name = faker.pystr()
+#     assert await _is_locked(redis_client_sdk, lock_name) is False
+#     assert await redis_client_sdk.lock_value(lock_name) is None
+#     async with redis_client_sdk.lock_context(lock_name, lock_value=lock_data):
+#         assert await _is_locked(redis_client_sdk, lock_name) is True
+#         assert await redis_client_sdk.lock_value(lock_name) == lock_data
+#     assert await _is_locked(redis_client_sdk, lock_name) is False
+#     assert await redis_client_sdk.lock_value(lock_name) is None
+
+
+# async def test_lock_context_released_after_error(
+#     redis_client_sdk: RedisClientSDK, faker: Faker
+# ):
+#     lock_name = faker.pystr()
+
+#     assert await redis_client_sdk.lock_value(lock_name) is None
+
+#     with pytest.raises(RuntimeError):  # noqa: PT012
+#         async with redis_client_sdk.lock_context(lock_name):
+#             assert await redis_client_sdk.redis.get(lock_name) is not None
+#             msg = "Expected error"
+#             raise RuntimeError(msg)
+
+#     assert await redis_client_sdk.lock_value(lock_name) is None
+
+
+# async def test_lock_acquired_in_parallel_to_update_same_resource(
+#     with_short_default_redis_lock_ttl: None,
+#     get_redis_client_sdk: Callable[
+#         [RedisDatabase], AbstractAsyncContextManager[RedisClientSDK]
+#     ],
+#     faker: Faker,
+# ):
+#     INCREASE_OPERATIONS: Final[int] = 250
+#     INCREASE_BY: Final[int] = 10
+
+#     class RaceConditionCounter:
+#         def __init__(self):
+#             self.value: int = 0
+
+#         async def race_condition_increase(self, by: int) -> None:
+#             current_value = self.value
+#             current_value += by
+#             # most likely situation which creates issues
+#             await asyncio.sleep(redis_constants.DEFAULT_LOCK_TTL.total_seconds() / 2)
+#             self.value = current_value
+
+#     counter = RaceConditionCounter()
+#     lock_name: str = faker.pystr()
+#     # ensures it does nto time out before acquiring the lock
+#     time_for_all_inc_counter_calls_to_finish_s: float = (
+#         redis_constants.DEFAULT_LOCK_TTL.total_seconds() * INCREASE_OPERATIONS * 10
+#     )
+
+#     async def _inc_counter() -> None:
+#         async with get_redis_client_sdk(  # noqa: SIM117
+#             RedisDatabase.RESOURCES
+#         ) as redis_client_sdk:
+#             async with redis_client_sdk.lock_context(
+#                 lock_key=lock_name,
+#                 blocking=True,
+#                 blocking_timeout_s=time_for_all_inc_counter_calls_to_finish_s,
+#             ):
+#                 await counter.race_condition_increase(INCREASE_BY)
+
+#     await limited_gather(
+#         *(_inc_counter() for _ in range(INCREASE_OPERATIONS)), limit=15
+#     )
+#     assert counter.value == INCREASE_BY * INCREASE_OPERATIONS
 
 
 async def test_redis_client_sdks_manager(