@pcrespov reviews

Author: sanderegg

packages/service-library/src/servicelib/background_task_utils.py

@@ -33,17 +33,17 @@ def exclusive_periodic(
     """
 
     def _decorator(
-        func: Callable[P, Coroutine[Any, Any, None]],
+        coro: Callable[P, Coroutine[Any, Any, None]],
     ) -> Callable[P, Coroutine[Any, Any, None]]:
         @periodic(interval=retry_after)
         @exclusive(
             redis_client,
-            lock_key=f"lock:exclusive_periodic_task:{func.__name__}",
+            lock_key=f"lock:exclusive_periodic_task:{coro.__module__}.{coro.__name__}",
         )
         @periodic(interval=task_interval)
-        @functools.wraps(func)
+        @functools.wraps(coro)
         async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> None:
-            return await func(*args, **kwargs)
+            return await coro(*args, **kwargs)
 
         return _wrapper
 

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

@@ -22,7 +22,7 @@
 P = ParamSpec("P")
 R = TypeVar("R")
 
-_EXCLUSIVE_TASK_NAME: Final[str] = "exclusive/{func_name}"
+_EXCLUSIVE_TASK_NAME: Final[str] = "exclusive/{module_name}.{func_name}"
 _EXCLUSIVE_AUTO_EXTEND_TASK_NAME: Final[
     str
 ] = "exclusive/autoextend_lock_{redis_lock_key}"
@@ -65,9 +65,9 @@ def exclusive(
         raise ValueError(msg)
 
     def _decorator(
-        func: Callable[P, Coroutine[Any, Any, R]],
+        coro: Callable[P, Coroutine[Any, Any, R]],
     ) -> Callable[P, Coroutine[Any, Any, R]]:
-        @functools.wraps(func)
+        @functools.wraps(coro)
         async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
             redis_lock_key = (
                 lock_key(*args, **kwargs) if callable(lock_key) else lock_key
@@ -109,10 +109,12 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
                     await started_event.wait()
 
                     # then the task that runs the user code
-                    assert asyncio.iscoroutinefunction(func)  # nosec
+                    assert asyncio.iscoroutinefunction(coro)  # nosec
                     work_task = tg.create_task(
-                        func(*args, **kwargs),
-                        name=_EXCLUSIVE_TASK_NAME.format(func_name=func.__name__),
+                        coro(*args, **kwargs),
+                        name=_EXCLUSIVE_TASK_NAME.format(
+                            module_name=coro.__module__, func_name=coro.__name__
+                        ),
                     )
 
                     res = await work_task
@@ -130,12 +132,6 @@ async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
 
                 assert lock_lost_errors is not None  # nosec
                 assert len(lock_lost_errors.exceptions) == 1  # nosec
-                _logger.error(  # noqa: TRY400
-                    "lock %s could not be auto-extended! "
-                    "TIP: check connection to Redis DBs or look for Synchronous "
-                    "code that might block the auto-extender task. Somehow the distributed lock disappeared!",
-                    lock.name,
-                )
                 raise lock_lost_errors.exceptions[0] from eg
             finally:
                 with contextlib.suppress(redis.exceptions.LockNotOwnedError):

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

@@ -14,4 +14,8 @@ class CouldNotConnectToRedisError(BaseRedisError):
 
 
 class LockLostError(BaseRedisError):
-    msg_template: str = "Lock {lock.name} has been lost"
+    msg_template: str = (
+        "Lock {lock.name} has been lost (e.g. it could not be auto-extended!)"
+        "TIP: check connection to Redis DBs or look for Synchronous "
+        "code that might block the auto-extender task. Somehow the distributed lock disappeared!"
+    )

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

@@ -31,7 +31,9 @@ def _assert_exclusive_tasks_are_cancelled(lock_name: str, func: Callable) -> Non
     assert _EXCLUSIVE_AUTO_EXTEND_TASK_NAME.format(redis_lock_key=lock_name) not in [
         t.get_name() for t in asyncio.tasks.all_tasks()
     ], "the auto extend lock task was not properly stopped!"
-    assert _EXCLUSIVE_TASK_NAME.format(func_name=func.__name__) not in [
+    assert _EXCLUSIVE_TASK_NAME.format(
+        module_name=func.__module__, func_name=func.__name__
+    ) not in [
         t.get_name() for t in asyncio.tasks.all_tasks()
     ], "the exclusive task was not properly stopped!"