search

monospice / laravel-redis-sentinel-drivers

Redis Sentinel integration for Laravel and Lumen.
MIT License
101 stars 48 forks source link

Add PredisSentinelConnection that provides transaction implementation… #12

Closed pdbreen closed 6 years ago

pdbreen commented 6 years ago

… that works w/aggregate connections. Taylor was being pretty rigid in rejecting PR's that would remove the transaction from Horizon. So, rather than attempt a framework PR that could see similar scrutiny, I thought I'd send this your way. Essentially, this converts any transaction() invocation that current Predis can't handle into a MULTI / EXEC which Predis does handle.

This is modeled on the PhpRedisConnection::transaction() implementation (unchanged since 5.4).

I can't vouch for Lumen compatibility, but better chance of working than guaranteed exception!

cyrossignol commented 6 years ago

@pdbreen Many thanks for all the work and research you've done here. I've considered this approach before, but I've been hesitant to change the behavior of transaction() for Predis connections because Predis' abstraction over MULTI/EXEC adds a lot more functionality than Laravel's implementation of transaction() for PhpRedis.

Granted—transactions simply don't work for Sentinel with the current version of Predis either way, so now seems as good a time as any to get this in!