[Messenger] Fix support for Redis Sentinel using php-redis 6.0.0 #52629
Conversation
| @@ -94,7 +94,21 @@ public function __construct(array $options, \Redis|Relay|\RedisCluster $redis = | |||
| } else { | |||
| if (null !== $sentinelMaster) { | |||
| $sentinelClass = \extension_loaded('redis') ? \RedisSentinel::class : Sentinel::class; | |||
| $sentinelClient = new $sentinelClass($host, $port, $options['timeout'], $options['persistent_id'], $options['retry_interval'], $options['read_timeout']); | |||
|
|
|||
| if (version_compare(phpversion('redis'), '6.0.0', '>=')) { | |||
There was a problem hiding this comment.
| if (version_compare(phpversion('redis'), '6.0.0', '>=')) { | |
| if (\extension_loaded('redis') && version_compare(phpversion('redis'), '6.0.0', '>=')) { |
There was a problem hiding this comment.
is there a something similar to do for Relay? /cc @tillkruss @michael-grunder.
There was a problem hiding this comment.
Good question. As of Relay v0.6.8 we support both (PhpRedis 5.x and 6.x) constructors.
So you could do the same check for Relay, simply to future proof it. I don't think we'll drop support for the 5.x-style constructor anytime soon, tho.
There was a problem hiding this comment.
@nicolas-grekas: There was another breaking change in PhpRedis 6.0 that I discovered randomly last week that can easily slip through the cracks and is hard to test for regarding flushdb() and flushall().
In PhpRedis 5.x flushdb(true) will trigger FLUSHDB ASYNC, while in PhpRedis 6.x flushdb(false) will trigger FLUSHDB ASYNC.
There was a problem hiding this comment.
Reversing the meaning of a boolean parameter is quite bad as BC break as it is not discoverable at all 😢
There was a problem hiding this comment.
That's bad for end users indeed...
There was a problem hiding this comment.
Yeah, and not even marking it as a BC in the release notes. I've complained to Michael and Pavlo about this. I discovered this by pure luck.
nicolas-grekas
force-pushed
the
fix_52487
branch
from
61a8072
to
42a80db
Compare
Added support for php-redis 6.0.0 to Symfony/Component/Messenger/Bridge/Redis/Transport/Connection.
A similar fix was done for RedisTrait before: #51683