Documentation bug in usage example

[mrmachine]

This example

python-redis-lock/README.rst

Lines 103 to 114 in 1497012

	import socket
	host_id = "owned-by-%s" % socket.gethostname()
	lock = redis_lock.Lock(conn, "name-of-the-lock", id=host_id)
	if lock.acquire(blocking=False):
	assert lock.locked() is True
	print("Got the lock.")
	lock.release()
	else:
	if lock.get_owner_id() == host_id:
	print("I already acquired this in another process.")
	else:
	print("The lock is held on another machine.")

has an unreachable code path and implies an impossible use case.

The example implies that a lock with an explicit ID can return False from .acquire(blocking=False) AND have the lock owned by self per lock.get_owner_id() == host_id check, which is actually just the same check as _held property, which will instead cause .acquire(blocking=False) to raise an exception.

Ideally I would like to be able to give locks in multiple processes the same ID and be able to call .acquire(blocking=False) and have it return False instead of raising AlreadyAcquired. I thought that was possible from reading the examples.

Then when I got AlreadyAcquired I had to change my code to try .acquire(): except AlreadyAcquired instead of if .acquire(), which is fine, except that the example further confused me by implying I might need to support both False or AlreadyAcquired in the event that I am unable to acquire the lock.

[acdameli]

Yeah this was also confusing to me. It appears as though there's no way to use a Lock instance as a context manager if you want to be able to reacquire a lock given the current actor is "allowed" to resume.

My use case is:

1. celery task starts doing some long running work
2. celery task acquires a lock using its id as the owner (if you can't acquire the lock you retry with a delay)
3. celery task starts doing its work
4. celery task is killed
5. celery task restarts w/ same id
6. celery task reacquires the lock using its same id as the owner
7. Problem:
   • Expected: lock is reacquired and everything acts like normal
   • Actual: AlreadyAcquired is raised

Arguably I'm just misusing the lib and this can be implemented with the try/acquire(blocking=False)/except just felt like context manager was intended to be used this way.

My hacky "fix" for this is the naive:

class ReacquirableLock(Lock):
    def __enter__(self):
        try:
            super().__enter__()
        except AlreadyAcquired:
            if self._lock_renewal_interval is not None:
                self._start_lock_renewer()
        return self