Skip to content

Commit 184af84

Browse files
author
Gabriel Erzse
committed
Handle RESP3 sets as Python lists
Although the RESP3 protocol defines the set data structure, sometimes the responses from the Redis server contain sets with nested maps, which cannot be represented in Python as sets with nested dicts, because dicts are not hashable. Versions of HIREDIS before 3.0.0 would cause segmentation fault when parsing such responses. Starting with version 3.0.0 the problem was fixed, with the compromise that RESP3 sets are represented as Python lists. The embedded RESP3 parser was so far trying to represent RESP3 sets as Python sets, if possible. Only when this was not possible it would switch to the list representation. Arguably this is not the best user experience, not knowing when you will get back a set or a list. Upgrade the required hiredis-py version to be at least 3.0.0, and change the embedded parser to always represent RESP3 sets as lists. This way we get a consistent experience in all cases. This is a breaking change.
1 parent 2ffcac3 commit 184af84

File tree

14 files changed

+51
-91
lines changed

14 files changed

+51
-91
lines changed

‎redis/_parsers/resp3.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,11 @@ def _read_response(self, disable_decoding=False, push_request=False):
8888
# set response
8989
elif byte == b"~":
9090
# redis can return unhashable types (like dict) in a set,
91-
# so we need to first convert to a list, and then try to convert it to a set
91+
# so we return sets as list, all the time, for predictability
9292
response = [
9393
self._read_response(disable_decoding=disable_decoding)
9494
for _ in range(int(response))
9595
]
96-
try:
97-
response = set(response)
98-
except TypeError:
99-
pass
10096
# map response
10197
elif byte == b"%":
10298
# We cannot use a dict-comprehension to parse stream.

‎redis/commands/bf/commands.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
from redis.client import NEVER_DECODE
2-
from redis.exceptions import ModuleError
3-
from redis.utils import HIREDIS_AVAILABLE, deprecated_function
2+
from redis.utils import deprecated_function
43

54
BF_RESERVE = "BF.RESERVE"
65
BF_ADD = "BF.ADD"
@@ -139,9 +138,6 @@ def scandump(self, key, iter):
139138
This command will return successive (iter, data) pairs until (0, NULL) to indicate completion.
140139
For more information see `BF.SCANDUMP <https://redis.io/commands/bf.scandump>`_.
141140
""" # noqa
142-
if HIREDIS_AVAILABLE:
143-
raise ModuleError("This command cannot be used when hiredis is available.")
144-
145141
params = [key, iter]
146142
options = {}
147143
options[NEVER_DECODE] = []

‎redis/utils.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
try:
77
import hiredis # noqa
88

9-
# Only support Hiredis >= 1.0:
10-
HIREDIS_AVAILABLE = not hiredis.__version__.startswith("0.")
9+
# Only support Hiredis >= 3.0:
10+
HIREDIS_AVAILABLE = int(hiredis.__version__.split('.')[0]) >= 3
1111
HIREDIS_PACK_AVAILABLE = hasattr(hiredis, "pack_command")
1212
except ImportError:
1313
HIREDIS_AVAILABLE = False

‎setup.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"Programming Language :: Python :: Implementation :: PyPy",
5757
],
5858
extras_require={
59-
"hiredis": ["hiredis>=1.0.0"],
59+
"hiredis": ["hiredis>=3.0.0"],
6060
"ocsp": ["cryptography>=36.0.1", "pyopenssl==23.2.1", "requests>=2.31.0"],
6161
},
6262
)

‎tests/test_asyncio/test_bloom.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ async def do_verify():
105105

106106
await do_verify()
107107
cmds = []
108-
if HIREDIS_AVAILABLE:
109-
with pytest.raises(ModuleError):
110-
cur = await decoded_r.bf().scandump("myBloom", 0)
111-
return
112108

113109
cur = await decoded_r.bf().scandump("myBloom", 0)
114110
first = cur[0]

‎tests/test_asyncio/test_commands.py

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,7 +1406,7 @@ async def test_zscan_iter(self, r: redis.Redis):
14061406
async def test_sadd(self, r: redis.Redis):
14071407
members = {b"1", b"2", b"3"}
14081408
await r.sadd("a", *members)
1409-
assert await r.smembers("a") == members
1409+
assert set(await r.smembers("a")) == members
14101410

14111411
async def test_scard(self, r: redis.Redis):
14121412
await r.sadd("a", "1", "2", "3")
@@ -1415,34 +1415,34 @@ async def test_scard(self, r: redis.Redis):
14151415
@pytest.mark.onlynoncluster
14161416
async def test_sdiff(self, r: redis.Redis):
14171417
await r.sadd("a", "1", "2", "3")
1418-
assert await r.sdiff("a", "b") == {b"1", b"2", b"3"}
1418+
assert set(await r.sdiff("a", "b")) == {b"1", b"2", b"3"}
14191419
await r.sadd("b", "2", "3")
1420-
assert await r.sdiff("a", "b") == {b"1"}
1420+
assert await r.sdiff("a", "b") == [b"1"]
14211421

14221422
@pytest.mark.onlynoncluster
14231423
async def test_sdiffstore(self, r: redis.Redis):
14241424
await r.sadd("a", "1", "2", "3")
14251425
assert await r.sdiffstore("c", "a", "b") == 3
1426-
assert await r.smembers("c") == {b"1", b"2", b"3"}
1426+
assert set(await r.smembers("c")) == {b"1", b"2", b"3"}
14271427
await r.sadd("b", "2", "3")
14281428
assert await r.sdiffstore("c", "a", "b") == 1
1429-
assert await r.smembers("c") == {b"1"}
1429+
assert await r.smembers("c") == [b"1"]
14301430

14311431
@pytest.mark.onlynoncluster
14321432
async def test_sinter(self, r: redis.Redis):
14331433
await r.sadd("a", "1", "2", "3")
1434-
assert await r.sinter("a", "b") == set()
1434+
assert await r.sinter("a", "b") == []
14351435
await r.sadd("b", "2", "3")
1436-
assert await r.sinter("a", "b") == {b"2", b"3"}
1436+
assert set(await r.sinter("a", "b")) == {b"2", b"3"}
14371437

14381438
@pytest.mark.onlynoncluster
14391439
async def test_sinterstore(self, r: redis.Redis):
14401440
await r.sadd("a", "1", "2", "3")
14411441
assert await r.sinterstore("c", "a", "b") == 0
1442-
assert await r.smembers("c") == set()
1442+
assert await r.smembers("c") == []
14431443
await r.sadd("b", "2", "3")
14441444
assert await r.sinterstore("c", "a", "b") == 2
1445-
assert await r.smembers("c") == {b"2", b"3"}
1445+
assert set(await r.smembers("c")) == {b"2", b"3"}
14461446

14471447
async def test_sismember(self, r: redis.Redis):
14481448
await r.sadd("a", "1", "2", "3")
@@ -1453,22 +1453,22 @@ async def test_sismember(self, r: redis.Redis):
14531453

14541454
async def test_smembers(self, r: redis.Redis):
14551455
await r.sadd("a", "1", "2", "3")
1456-
assert await r.smembers("a") == {b"1", b"2", b"3"}
1456+
assert set(await r.smembers("a")) == {b"1", b"2", b"3"}
14571457

14581458
@pytest.mark.onlynoncluster
14591459
async def test_smove(self, r: redis.Redis):
14601460
await r.sadd("a", "a1", "a2")
14611461
await r.sadd("b", "b1", "b2")
14621462
assert await r.smove("a", "b", "a1")
1463-
assert await r.smembers("a") == {b"a2"}
1464-
assert await r.smembers("b") == {b"b1", b"b2", b"a1"}
1463+
assert await r.smembers("a") == [b"a2"]
1464+
assert set(await r.smembers("b")) == {b"b1", b"b2", b"a1"}
14651465

14661466
async def test_spop(self, r: redis.Redis):
14671467
s = [b"1", b"2", b"3"]
14681468
await r.sadd("a", *s)
14691469
value = await r.spop("a")
14701470
assert value in s
1471-
assert await r.smembers("a") == set(s) - {value}
1471+
assert set(await r.smembers("a")) == set(s) - {value}
14721472

14731473
@skip_if_server_version_lt("3.2.0")
14741474
async def test_spop_multi_value(self, r: redis.Redis):
@@ -1481,9 +1481,7 @@ async def test_spop_multi_value(self, r: redis.Redis):
14811481
assert value in s
14821482

14831483
response = await r.spop("a", 1)
1484-
assert_resp_response(
1485-
r, response, list(set(s) - set(values)), set(s) - set(values)
1486-
)
1484+
assert set(response) == set(s) - set(values)
14871485

14881486
async def test_srandmember(self, r: redis.Redis):
14891487
s = [b"1", b"2", b"3"]
@@ -1502,20 +1500,20 @@ async def test_srem(self, r: redis.Redis):
15021500
await r.sadd("a", "1", "2", "3", "4")
15031501
assert await r.srem("a", "5") == 0
15041502
assert await r.srem("a", "2", "4") == 2
1505-
assert await r.smembers("a") == {b"1", b"3"}
1503+
assert set(await r.smembers("a")) == {b"1", b"3"}
15061504

15071505
@pytest.mark.onlynoncluster
15081506
async def test_sunion(self, r: redis.Redis):
15091507
await r.sadd("a", "1", "2")
15101508
await r.sadd("b", "2", "3")
1511-
assert await r.sunion("a", "b") == {b"1", b"2", b"3"}
1509+
assert set(await r.sunion("a", "b")) == {b"1", b"2", b"3"}
15121510

15131511
@pytest.mark.onlynoncluster
15141512
async def test_sunionstore(self, r: redis.Redis):
15151513
await r.sadd("a", "1", "2")
15161514
await r.sadd("b", "2", "3")
15171515
assert await r.sunionstore("c", "a", "b") == 3
1518-
assert await r.smembers("c") == {b"1", b"2", b"3"}
1516+
assert set(await r.smembers("c")) == {b"1", b"2", b"3"}
15191517

15201518
# SORTED SET COMMANDS
15211519
async def test_zadd(self, r: redis.Redis):

‎tests/test_asyncio/test_search.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ async def test_tags(decoded_r: redis.Redis):
862862
assert 1 == res["total_results"]
863863

864864
q2 = await decoded_r.ft().tagvals("tags")
865-
assert set(tags.split(",") + tags2.split(",")) == q2
865+
assert set(tags.split(",") + tags2.split(",")) == set(q2)
866866

867867

868868
@pytest.mark.redismod
@@ -986,7 +986,7 @@ async def test_dict_operations(decoded_r: redis.Redis):
986986

987987
# Dump dict and inspect content
988988
res = await decoded_r.ft().dict_dump("custom_dict")
989-
assert_resp_response(decoded_r, res, ["item1", "item3"], {"item1", "item3"})
989+
assert res == ["item1", "item3"]
990990

991991
# Remove rest of the items before reload
992992
await decoded_r.ft().dict_del("custom_dict", *res)

‎tests/test_asyncio/test_timeseries.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -745,9 +745,7 @@ async def test_query_index(decoded_r: redis.Redis):
745745
await decoded_r.ts().create(2, labels={"Test": "This", "Taste": "That"})
746746
assert 2 == len(await decoded_r.ts().queryindex(["Test=This"]))
747747
assert 1 == len(await decoded_r.ts().queryindex(["Taste=That"]))
748-
assert_resp_response(
749-
decoded_r, await decoded_r.ts().queryindex(["Taste=That"]), [2], {"2"}
750-
)
748+
assert ["2"] == await decoded_r.ts().queryindex(["Taste=That"])
751749

752750

753751
@pytest.mark.redismod

‎tests/test_bloom.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,6 @@ def do_verify():
136136

137137
do_verify()
138138
cmds = []
139-
if HIREDIS_AVAILABLE:
140-
with pytest.raises(ModuleError):
141-
cur = client.bf().scandump("myBloom", 0)
142-
return
143139

144140
cur = client.bf().scandump("myBloom", 0)
145141
first = cur[0]

‎tests/test_command_parser.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import pytest
22
from redis._parsers import CommandsParser
3-
from redis.utils import HIREDIS_AVAILABLE
43

54
from .conftest import (
65
assert_resp_response,
@@ -9,9 +8,6 @@
98
)
109

1110

12-
# The response to COMMAND contains maps inside sets, which are not handled
13-
# by the hiredis-py parser (see https://github.com/redis/hiredis-py/issues/188)
14-
@pytest.mark.skipif(HIREDIS_AVAILABLE, reason="PythonParser only")
1511
class TestCommandsParser:
1612
def test_init_commands(self, r):
1713
commands_parser = CommandsParser(r)

0 commit comments

Comments
 (0)