redis
Typing: `Lock.extend` and `Lock.do_extend` could return `Literal[True]`
Hi there! I ran into this recently: both the sync and async Lock APIs have extend and do_extend typed as returning bool, but in practice only True is returned.
For example, here:
https://github.com/redis/redis-py/blob/b96d29c40ffa96e75330a933023f2f8eacd5ce64/redis/lock.py#L287-L317
Instead of False, these APIs use exceptions to communicate failure states.
The current return type can lead to developer confusion, e.g. developers mistakenly doing this:
if not lock.extend(...):
# handle lock extension failure
...which doesn't work as expected, since lock failure causes a raise LockError instead.
Proposed solution
I think the simplest solution here would be to refine the return types on these APIs: instead of -> bool these APIs could be annotated with -> typing.Literal[True] to communicate clearly to developers that there's no meaningful conditional check on the return value.
Alternatives
An alternative to the proposed solution above would be to document in redis-py's docs that the return type of bool is really just the True value, and that users must handle errors solely through exception handling. I think this would be fine, but that it would be a lot clearer to have this encoded at the type layer additionally, per above 🙂
It might even be better to just return None since the True has no semantic meaning, though that's probably breaking.
Hi @zanieb, thank you for reporting this problem! We will have a look at it.
