test passes

Author: sanderegg

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

@@ -8,14 +8,10 @@
 
 import redis.exceptions
 from redis.asyncio.lock import Lock
-from tenacity import retry
-
-from servicelib.async_utils import cancel_wait_task, with_delay
-from servicelib.logging_utils import log_context
 
 from ..background_task import periodic
 from ._client import RedisClientSDK
-from ._constants import DEFAULT_LOCK_TTL, SHUTDOWN_TIMEOUT_S
+from ._constants import DEFAULT_LOCK_TTL
 from ._errors import CouldNotAcquireLockError, LockLostError
 from ._utils import auto_extend_lock
 
@@ -25,30 +21,15 @@
 R = TypeVar("R")
 
 _EXCLUSIVE_TASK_NAME: Final[str] = "exclusive/{func_name}"
-_EXCLUSIVE_AUTO_EXTEND_TASK_NAME: Final[str] = (
-    "exclusive/autoextend_lock_{redis_lock_key}"
-)
+_EXCLUSIVE_AUTO_EXTEND_TASK_NAME: Final[
+    str
+] = "exclusive/autoextend_lock_{redis_lock_key}"
 
 
 @periodic(interval=DEFAULT_LOCK_TTL / 2, raise_on_error=True)
 async def _periodic_auto_extender(lock: Lock, started_event: asyncio.Event) -> None:
-    started_event.set()
     await auto_extend_lock(lock)
-    current_task = asyncio.tasks.current_task()
-    assert current_task is not None  # nosec
-    print(current_task.cancelling())
-
-
-def _cancel_auto_extender_task(
-    _: asyncio.Task, *, auto_extend_task: asyncio.Task
-) -> None:
-    with log_context(
-        _logger,
-        logging.DEBUG,
-        f"Cancelling auto-extend task {auto_extend_task.get_name()}",
-    ):
-        auto_extend_task.cancel()
-        assert auto_extend_task.cancelling()
+    started_event.set()
 
 
 def exclusive(
@@ -101,9 +82,9 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
             if not await lock.acquire(
                 token=lock_value,
                 blocking=blocking,
-                blocking_timeout=blocking_timeout.total_seconds()
-                if blocking_timeout
-                else None,
+                blocking_timeout=(
+                    blocking_timeout.total_seconds() if blocking_timeout else None
+                ),
             ):
                 raise CouldNotAcquireLockError(lock=lock)
 
@@ -117,7 +98,8 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
                             redis_lock_key=redis_lock_key
                         ),
                     )
-                    # NOTE: in case the work task is super short lived, then we might fail in cancelling it
+                    # NOTE: In case the work thread is raising right away,
+                    # this ensures the extend task ran once and ensure cancellation works
                     await started_event.wait()
 
                     # then the task that runs the user code
@@ -127,20 +109,11 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
                         name=_EXCLUSIVE_TASK_NAME.format(func_name=func.__name__),
                     )
 
-                    # work_task.add_done_callback(
-                    #     functools.partial(
-                    #         _cancel_auto_extender_task,
-                    #         auto_extend_task=auto_extend_lock_task,
-                    #     )
-                    # )
-
                     res = await work_task
                     auto_extend_lock_task.cancel()
-
                     return res
 
             except BaseExceptionGroup as eg:
-                breakpoint()
                 # Separate exceptions into LockLostError and others
                 lock_lost_errors, other_errors = eg.split(LockLostError)
 
@@ -158,8 +131,6 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
                     lock.name,
                 )
                 raise lock_lost_errors.exceptions[0] from eg
-            except Exception as exc:
-                breakpoint()
             finally:
                 with contextlib.suppress(redis.exceptions.LockNotOwnedError):
                     # in the case where the lock would have been lost,

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

@@ -12,8 +12,8 @@
 import pytest
 from faker import Faker
 from servicelib.redis import CouldNotAcquireLockError, RedisClientSDK, exclusive
-from servicelib.redis._errors import LockLostError
 from servicelib.redis._decorators import _EXCLUSIVE_AUTO_EXTEND_TASK_NAME
+from servicelib.redis._errors import LockLostError
 from servicelib.utils import limited_gather, logged_gather
 
 pytest_simcore_core_services_selection = [
@@ -214,19 +214,13 @@ async def test_exclusive_task_erroring_releases_lock(
 ):
     @exclusive(redis_client_sdk, lock_key=lock_name)
     async def _() -> None:
-        # await asyncio.sleep(0)
-
-        print("raising error")
         msg = "Expected error"
         raise RuntimeError(msg)
 
     # initial state
     assert await _is_locked(redis_client_sdk, lock_name) is False
     assert await redis_client_sdk.lock_value(lock_name) is None
 
-    # run the exclusive task
-    # exclusive_task = asyncio.create_task(_())
-
     with pytest.raises(RuntimeError):
         await _()