Skip to content

Commit

Permalink
Robustly test if Redis clients talk to the same db (#560)
Browse files Browse the repository at this point in the history
* Robustly test if Redis clients talk to the same db

Previously, we were comparing the two clients' `connection_kwargs`, but
`connection_kwargs` contains more than just `host`, `port`, and `db`:

```python
>>> from redis import Redis
>>> redis = Redis()
>>> redis.connection_pool.connection_kwargs
{'db': 0, 'username': None, 'password': None, 'socket_timeout': None, 'encoding': 'utf-8', 'encoding_errors': 'strict', 'decode_responses': False, 'retry_on_error': [], 'retry': None, 'health_check_interval': 0, 'client_name': None, 'redis_connect_func': None, 'host': 'localhost', 'port': 6379, 'socket_connect_timeout': None, 'socket_keepalive': None, 'socket_keepalive_options': None}
```

This PR allows Pottery to recognize that two Redis clients are connected
to the same database even if their socket timeout our retry policies are
different.

* Bump version number
  • Loading branch information
brainix authored Dec 29, 2021
1 parent 687d41a commit 2a1a9e0
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 69 deletions.
2 changes: 1 addition & 1 deletion pottery/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@


__title__: Final[str] = 'pottery'
__version__: Final[str] = '2.3.0'
__version__: Final[str] = '2.3.1'
__description__: Final[str] = __doc__.split(sep='\n\n', maxsplit=1)[0]
__url__: Final[str] = 'https://github.com/brainix/pottery'
__author__: Final[str] = 'Rajiv Bakulesh Shah'
Expand Down
17 changes: 10 additions & 7 deletions pottery/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ def random_key(*,
return key


def connection_args(redis: Redis) -> Tuple[str, int, int]:
return (
redis.connection_pool.connection_kwargs['host'],
redis.connection_pool.connection_kwargs.get('port', 6379),
redis.connection_pool.connection_kwargs.get('db', 0),
)


class _Common:
'Mixin class that implements self.redis and self.key properties.'

Expand Down Expand Up @@ -213,12 +221,7 @@ def __context_managers(self,
redises = collections.defaultdict(list)
for container in (self, *others):
if isinstance(container, _Pipelined):
connection_kwargs = (
container.redis.connection_pool.connection_kwargs['host'],
container.redis.connection_pool.connection_kwargs.get('port', 6379),
container.redis.connection_pool.connection_kwargs.get('db', 0),
)
redises[connection_kwargs].append(container)
redises[connection_args(container.redis)].append(container)
for containers in redises.values():
keys = (container.key for container in containers)
pipeline = containers[0].__watch_keys(*keys)
Expand Down Expand Up @@ -262,7 +265,7 @@ def _same_redis(self, *others: Any) -> bool:
for other in others:
if not isinstance(other, _Comparable):
return False
if self.redis.connection_pool != other.redis.connection_pool:
if connection_args(self.redis) != connection_args(other.redis):
return False
return True

Expand Down
3 changes: 2 additions & 1 deletion pottery/hyper.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from .annotations import RedisValues
from .base import Base
from .base import JSONTypes
from .base import connection_args
from .base import random_key


Expand Down Expand Up @@ -131,7 +132,7 @@ def update(self,
with self._watch(objs) as pipeline:
for obj in objs:
if isinstance(obj, self.__class__):
if self.redis.connection_pool != obj.redis.connection_pool:
if connection_args(self.redis) != connection_args(obj.redis):
raise RuntimeError(
f"can't update {self} with {obj} as they live on "
"different Redis instances/databases"
Expand Down
22 changes: 0 additions & 22 deletions pottery/monkey.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,6 @@
_logger: Final[logging.Logger] = logging.getLogger('pottery')


# The Redis client connection pool doesn't have a sane equality test. So
# monkey patch equality comparisons on to the connection pool. We consider two
# connection pools to be equal if they're connected to the same host, port, and
# database.

from redis import ConnectionPool # isort: skip

def __eq__(self: ConnectionPool, other: Any) -> bool:
try:
value: bool = self.connection_kwargs == other.connection_kwargs
except AttributeError: # pragma: no cover
value = False
return value

ConnectionPool.__eq__ = __eq__ # type: ignore

_logger.info(
'Monkey patched redis.ConnectionPool.__eq__() to compare Redis clients by '
'connection params'
)


# Monkey patch the JSON encoder to be able to JSONify any instance of any class
# that defines a to_dict(), to_list(), or to_str() method (since the encoder
# already knows how to JSONify dicts, lists, and strings).
Expand Down
38 changes: 0 additions & 38 deletions tests/test_redis.py

This file was deleted.

0 comments on commit 2a1a9e0

Please sign in to comment.