Add equality test on Redis client and conn pool #1240
Conversation
Currently, there's no `__eq__()` method defined on the `Redis` or `ConnectionPool` classes. Therefore, no two instances of either of these classes will ever be equal. I have a use case where it would be nice for one Redis client instance to be equal to another if they have the same connection kwargs; that is, if they're connected to the same Redis database.
Codecov Report
@@ Coverage Diff @@
## master #1240 +/- ##
==========================================
+ Coverage 92.34% 92.41% +0.06%
==========================================
Files 20 21 +1
Lines 5958 6010 +52
==========================================
+ Hits 5502 5554 +52
Misses 456 456
Continue to review full report at Codecov.
|
As of this change, `connection_pool != None` and `redis != None` both work, and both return `False`.
brainix
force-pushed
the
eq-test-on-pools
branch
from
99e025a to
2f4e2ad
Compare
This is a more natural place to compare Redis connections. Then we can use `ConnectionPool.__eq__()` to implement `Redis.__eq__()`. This feels less hacky. I've submitted this change to `redis-py`: redis/redis-py#1240 If it gets merged upstream, then I'll be able to delete this monkey patch.
This is a more natural place to compare Redis connections. Then we can use `ConnectionPool.__eq__()` to implement `Redis.__eq__()`. This feels less hacky. I've submitted this change to `redis-py`: redis/redis-py#1240 If it gets merged upstream, then I'll be able to delete this monkey patch.
* Add __eq__() method on ConnectionPool class This is a more natural place to compare Redis connections. Then we can use `ConnectionPool.__eq__()` to implement `Redis.__eq__()`. This feels less hacky. I've submitted this change to `redis-py`: redis/redis-py#1240 If it gets merged upstream, then I'll be able to delete this monkey patch. * Comment on PR submitted to redis-py
|
Looks good to me, thanks! |
|
@brainix After thinking about this a bit more, I now believe this isn't the right way to go. Just because two connection pools share the same configuration doesn't make them identical. Clearly they manage a separate set of connections (to the same server). It seems wrong to say I'd tentatively be OK with saying I'm going to revert this change for now. If you really need this functionality it's trivial to compare |
After further thought this was a bad idea. Just because two connection pools share the same connection arguments does not make them equal. It would seem quite odd if pool_a == pool_b yet pool_a.disconnect() doesn't close all of pool_b's connections. Ref #1240 Fixes #1277 Fixes #1275 Fixes #1267 Fixes #1273
|
@andymccurdy - Thanks for the heads up! That works for me. |
Apparently, this broke Celery when I try to contribute it upstream, heh. Instead, monkey patch `__eq__()` on the `ConnectionPool` class. More context: redis/redis-py#1240 (comment)
Apparently, this broke Celery when I try to contribute it upstream, heh. Instead, monkey patch `__eq__()` on the `ConnectionPool` class. More context: redis/redis-py#1240 (comment)
Currently, there's no
__eq__()method defined on theRedisorConnectionPoolclasses. Therefore, no two instances of either ofthese classes will ever be equal.
I have a use case where it would be nice for one Redis client instance
to be equal to another if they have the same connection kwargs; that is,
if they're connected to the same Redis database.
Pull Request check-list
Please make sure to review and check all of these items:
$ python setup.py testpass with this change (including linting)?NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Add an
__eq__()method on theRedisandConnectionPoolclasses that return true if they have the same connection kwargs; that is, if they're connected to the same Redis database.