Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding lock_name to LockError #3023

Merged
merged 3 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
* Add an optional lock_name attribute to LockError.
* Fix return types for `get`, `set_path` and `strappend` in JSONCommands
* Connection.register_connect_callback() is made public.
* Fix async `read_response` to use `disable_decoding`.
Expand Down
5 changes: 4 additions & 1 deletion redis/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ class LockError(RedisError, ValueError):
"Errors acquiring or releasing a lock"
# NOTE: For backwards compatibility, this class derives from ValueError.
# This was originally chosen to behave like threading.Lock.
pass

def __init__(self, message, lock_name=None):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: message was an optional field before, would be nice to keep it optional here too as this change was released as a patch :)

self.message = message
self.lock_name = lock_name


class LockNotOwnedError(LockError):
Expand Down
33 changes: 24 additions & 9 deletions redis/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ def register_scripts(self) -> None:
def __enter__(self) -> "Lock":
if self.acquire():
return self
raise LockError("Unable to acquire lock within the time specified")
raise LockError(
"Unable to acquire lock within the time specified",
lock_name=self.name,
)

def __exit__(
self,
Expand Down Expand Up @@ -248,15 +251,18 @@ def release(self) -> None:
"""
expected_token = self.local.token
if expected_token is None:
raise LockError("Cannot release an unlocked lock")
raise LockError("Cannot release an unlocked lock", lock_name=self.name)
self.local.token = None
self.do_release(expected_token)

def do_release(self, expected_token: str) -> None:
if not bool(
self.lua_release(keys=[self.name], args=[expected_token], client=self.redis)
):
raise LockNotOwnedError("Cannot release a lock that's no longer owned")
raise LockNotOwnedError(
"Cannot release a lock that's no longer owned",
lock_name=self.name,
)

def extend(self, additional_time: int, replace_ttl: bool = False) -> bool:
"""
Expand All @@ -270,9 +276,9 @@ def extend(self, additional_time: int, replace_ttl: bool = False) -> bool:
`additional_time`.
"""
if self.local.token is None:
raise LockError("Cannot extend an unlocked lock")
raise LockError("Cannot extend an unlocked lock", lock_name=self.name)
if self.timeout is None:
raise LockError("Cannot extend a lock with no timeout")
raise LockError("Cannot extend a lock with no timeout", lock_name=self.name)
return self.do_extend(additional_time, replace_ttl)

def do_extend(self, additional_time: int, replace_ttl: bool) -> bool:
Expand All @@ -284,17 +290,23 @@ def do_extend(self, additional_time: int, replace_ttl: bool) -> bool:
client=self.redis,
)
):
raise LockNotOwnedError("Cannot extend a lock that's no longer owned")
raise LockNotOwnedError(
"Cannot extend a lock that's no longer owned",
lock_name=self.name,
)
return True

def reacquire(self) -> bool:
"""
Resets a TTL of an already acquired lock back to a timeout value.
"""
if self.local.token is None:
raise LockError("Cannot reacquire an unlocked lock")
raise LockError("Cannot reacquire an unlocked lock", lock_name=self.name)
if self.timeout is None:
raise LockError("Cannot reacquire a lock with no timeout")
raise LockError(
"Cannot reacquire a lock with no timeout",
lock_name=self.name,
)
return self.do_reacquire()

def do_reacquire(self) -> bool:
Expand All @@ -304,5 +316,8 @@ def do_reacquire(self) -> bool:
keys=[self.name], args=[self.local.token, timeout], client=self.redis
)
):
raise LockNotOwnedError("Cannot reacquire a lock that's no longer owned")
raise LockNotOwnedError(
"Cannot reacquire a lock that's no longer owned",
lock_name=self.name,
)
return True
7 changes: 7 additions & 0 deletions tests/test_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,13 @@ def test_context_manager_reacquiring_lock_no_longer_owned_raises_error(self, r):
with self.get_lock(r, "foo", timeout=10, blocking=False):
r.set("foo", "a")

def test_lock_error_gives_correct_lock_name(self, r):
r.set("foo", "bar")
with pytest.raises(LockError) as excinfo:
with self.get_lock(r, "foo", blocking_timeout=0.1):
pass
assert excinfo.value.lock_name == "foo"


class TestLockClassSelection:
def test_lock_class_argument(self, r):
Expand Down