Skip to content

Commit

Permalink
Implement feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ben-cutler-datarobot committed Aug 21, 2023
1 parent 144f825 commit b6474be
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 7 deletions.
8 changes: 8 additions & 0 deletions aiocache/backends/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ def __init__(
**kwargs,
):
super().__init__(**kwargs)

# NOTE: decoding can't be controlled on API level after switching to
# redis, we need to disable decoding on global/connection level
# (decode_responses=False), because some of the values are saved as
# bytes directly, like pickle serialized values, which may raise an
# exception when decoded with 'utf-8'.
if client.connection_pool.connection_kwargs['decode_responses']:
raise ValueError("redis client must be constructed with decode_responses set to False")
self.client = client

async def _get(self, key, encoding="utf-8", _conn=None):
Expand Down
2 changes: 2 additions & 0 deletions aiocache/factory.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy

Check notice

Code scanning / CodeQL

Module is imported with 'import' and 'import from' Note

Module 'copy' is imported with both 'import' and 'import from'.
import logging
import urllib
from copy import deepcopy
Expand All @@ -20,6 +21,7 @@ def _class_from_string(class_path):


def _create_cache(cache, serializer=None, plugins=None, **kwargs):
kwargs = copy.deepcopy(kwargs)
if serializer is not None:
cls = serializer.pop("class")
cls = _class_from_string(cls) if isinstance(cls, str) else cls
Expand Down
16 changes: 10 additions & 6 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@ def max_conns():
return None


@pytest.fixture()
def decode_responses():
return False


@pytest.fixture
async def redis_client(max_conns):
r = redis.Redis(
async def redis_client(max_conns, decode_responses):
async with redis.Redis(
host="127.0.0.1",
port=6379,
db=0,
password=None,
decode_responses=False,
decode_responses=decode_responses,
socket_connect_timeout=None,
max_connections=max_conns
)
yield r
await r.close()
) as r:
yield r
2 changes: 1 addition & 1 deletion tests/ut/backends/test_memcached.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_setup(self):

def test_setup_override(self):
with patch.object(aiomcache, "Client", autospec=True) as aiomcache_client:
memcached = MemcachedBackend(endpoint="127.0.0.2", port=2, pool_size=10)
memcached = MemcachedBackend(host="127.0.0.2", port=2, pool_size=10)

aiomcache_client.assert_called_with("127.0.0.2", 2, pool_size=10)

Expand Down
9 changes: 9 additions & 0 deletions tests/ut/backends/test_redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ def redis(redis_client):

class TestRedisBackend:

@pytest.mark.parametrize('decode_responses', [True])
async def test_redis_backend_requires_client_decode_responses(self, redis_client):
with pytest.raises(ValueError) as ve:
RedisBackend(client=redis_client)

assert str(ve.value) == (
"redis client must be constructed with decode_responses set to False"
)

async def test_get(self, redis):
redis.client.get.return_value = b"value"
assert await redis._get(Keys.KEY) == "value"
Expand Down

0 comments on commit b6474be

Please sign in to comment.