pipeline.watch runs transaction even if no commands are queued

[taylor-cedar]

If you modify a key after a pipeline.watch from the same client it throws a redis.WatchError. It should ignore changes from the current client. This bug makes it so we can't use redis.lock.Lock.

Here is a link to the lock implementation. #216

[bmerry]

That seems like expected behaviour to me:

import redis

r = redis.StrictRedis()
r.set('foo', 'a')
pipe = r.pipeline()
pipe.watch('foo')
pipe.set('foo', 'b')
pipe.multi()
pipe.set('bar', 'c')
print(pipe.execute())

raises a WatchError.

Do you have some sample code that behaves differently between fakeredis and redis-py?

[taylor-cedar]

Yes, sorry for not showing this originally. Out of the redis-py code base.
https://github.com/andymccurdy/redis-py/blob/master/redis/lock.py#L138

def execute_release(pipe):
  lock_value = pipe.get(name)
  if lock_value != expected_token:
    raise LockError("Cannot release a lock that's no longer owned")
   pipe.delete(name)

self.redis.transaction(execute_release, name)

Try setting a key first and then run this code. Inside transaction() a WatchError is thrown when pipe.delete(name) is called above which causes raise LockError("Cannot release a lock that's no longer owned"). It's not necessarily a bug with WatchError, but the transaction should not fail because the key is modified in the transaction.

[bmerry]

I'll need to step through the code to check, but I actually think that might be a bug in redis-py. It's using a watch, but isn't calling multi to start the transaction, which means that it's not actually transactional (i.e. it'll go through even if some other process changed the key after the get call. The difference with fakeredis is that real redis special-cases an empty transaction and doesn't do the MULTI/EXEC pair.

[taylor-cedar]

Hmm, you may be right, but it looks like redis-py transaction code is about the same as the fakeredis code, so I think there is some behavior difference there. Even if it's a bug in redis-py, we probably need to implement it the same, unfortunately. Or have you deviated in the past because of bugs?

[bmerry]

I think the simplest fix is going to be mirroring redis-py's behaviour of returning early from pipeline.execute() if there are no queued commands, without checking any of the watches. That is a difference from redis-py that's worth fixing anyway.

I've filed redis/redis-py#1031 for the redis-py bug.

[taylor-cedar]

Unfortunately redis-py is basically abandoned, so it's unlikely to get fixed. I have a PR open for almost a year that has not been reviewed. I created my own fork with fixes that were required.

[bmerry]

I created my own fork with fixes that were required.

Do you publish versions of it? That's pretty much how I ended up being the fakeredis maintainer: I published a fork and eventually the original maintainer asked if I'd like to take over.

[bmerry]

Edited the title to reflect the actual issue where fakeredis is different from redis-py.