Skip to content

Commit 49d9cb7

Browse files
dvora-hchayimjamestiotio
authored
Version 4.4.4 (#2671)
* Fixing cancelled async futures (#2666) Co-authored-by: James R T <[email protected]> Co-authored-by: dvora-h <[email protected]> * Version 4.4.4 * fix tests * linters * fixing the test for the 4.4 state * remove superfluous try-except --------- Co-authored-by: Chayim <[email protected]> Co-authored-by: James R T <[email protected]> Co-authored-by: Chayim I. Kirshen <[email protected]>
1 parent b3c89ac commit 49d9cb7

File tree

8 files changed

+227
-74
lines changed

8 files changed

+227
-74
lines changed

.github/workflows/integration.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ jobs:
5151
timeout-minutes: 30
5252
strategy:
5353
max-parallel: 15
54+
fail-fast: false
5455
matrix:
5556
python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', 'pypy-3.7', 'pypy-3.8']
5657
test-type: ['standalone', 'cluster']
@@ -108,6 +109,7 @@ jobs:
108109
name: Install package from commit hash
109110
runs-on: ubuntu-latest
110111
strategy:
112+
fail-fast: false
111113
matrix:
112114
python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', 'pypy-3.7']
113115
steps:

redis/asyncio/client.py

+61-27
Original file line numberDiff line numberDiff line change
@@ -493,24 +493,32 @@ async def _disconnect_raise(self, conn: Connection, error: Exception):
493493
):
494494
raise error
495495

496-
# COMMAND EXECUTION AND PROTOCOL PARSING
497-
async def execute_command(self, *args, **options):
498-
"""Execute a command and return a parsed response"""
499-
await self.initialize()
500-
pool = self.connection_pool
501-
command_name = args[0]
502-
conn = self.connection or await pool.get_connection(command_name, **options)
503-
496+
async def _try_send_command_parse_response(self, conn, *args, **options):
504497
try:
505498
return await conn.retry.call_with_retry(
506499
lambda: self._send_command_parse_response(
507-
conn, command_name, *args, **options
500+
conn, args[0], *args, **options
508501
),
509502
lambda error: self._disconnect_raise(conn, error),
510503
)
504+
except asyncio.CancelledError:
505+
await conn.disconnect(nowait=True)
506+
raise
511507
finally:
512508
if not self.connection:
513-
await pool.release(conn)
509+
await self.connection_pool.release(conn)
510+
511+
# COMMAND EXECUTION AND PROTOCOL PARSING
512+
async def execute_command(self, *args, **options):
513+
"""Execute a command and return a parsed response"""
514+
await self.initialize()
515+
pool = self.connection_pool
516+
command_name = args[0]
517+
conn = self.connection or await pool.get_connection(command_name, **options)
518+
519+
return await asyncio.shield(
520+
self._try_send_command_parse_response(conn, *args, **options)
521+
)
514522

515523
async def parse_response(
516524
self, connection: Connection, command_name: Union[str, bytes], **options
@@ -749,10 +757,18 @@ async def _disconnect_raise_connect(self, conn, error):
749757
is not a TimeoutError. Otherwise, try to reconnect
750758
"""
751759
await conn.disconnect()
760+
752761
if not (conn.retry_on_timeout and isinstance(error, TimeoutError)):
753762
raise error
754763
await conn.connect()
755764

765+
async def _try_execute(self, conn, command, *arg, **kwargs):
766+
try:
767+
return await command(*arg, **kwargs)
768+
except asyncio.CancelledError:
769+
await conn.disconnect()
770+
raise
771+
756772
async def _execute(self, conn, command, *args, **kwargs):
757773
"""
758774
Connect manually upon disconnection. If the Redis server is down,
@@ -761,9 +777,11 @@ async def _execute(self, conn, command, *args, **kwargs):
761777
called by the # connection to resubscribe us to any channels and
762778
patterns we were previously listening to
763779
"""
764-
return await conn.retry.call_with_retry(
765-
lambda: command(*args, **kwargs),
766-
lambda error: self._disconnect_raise_connect(conn, error),
780+
return await asyncio.shield(
781+
conn.retry.call_with_retry(
782+
lambda: self._try_execute(conn, command, *args, **kwargs),
783+
lambda error: self._disconnect_raise_connect(conn, error),
784+
)
767785
)
768786

769787
async def parse_response(self, block: bool = True, timeout: float = 0):
@@ -1165,6 +1183,18 @@ async def _disconnect_reset_raise(self, conn, error):
11651183
await self.reset()
11661184
raise
11671185

1186+
async def _try_send_command_parse_response(self, conn, *args, **options):
1187+
try:
1188+
return await conn.retry.call_with_retry(
1189+
lambda: self._send_command_parse_response(
1190+
conn, args[0], *args, **options
1191+
),
1192+
lambda error: self._disconnect_reset_raise(conn, error),
1193+
)
1194+
except asyncio.CancelledError:
1195+
await conn.disconnect()
1196+
raise
1197+
11681198
async def immediate_execute_command(self, *args, **options):
11691199
"""
11701200
Execute a command immediately, but don't auto-retry on a
@@ -1180,12 +1210,8 @@ async def immediate_execute_command(self, *args, **options):
11801210
command_name, self.shard_hint
11811211
)
11821212
self.connection = conn
1183-
1184-
return await conn.retry.call_with_retry(
1185-
lambda: self._send_command_parse_response(
1186-
conn, command_name, *args, **options
1187-
),
1188-
lambda error: self._disconnect_reset_raise(conn, error),
1213+
return await asyncio.shield(
1214+
self._try_send_command_parse_response(conn, *args, **options)
11891215
)
11901216

11911217
def pipeline_execute_command(self, *args, **options):
@@ -1353,6 +1379,19 @@ async def _disconnect_raise_reset(self, conn: Connection, error: Exception):
13531379
await self.reset()
13541380
raise
13551381

1382+
async def _try_execute(self, conn, execute, stack, raise_on_error):
1383+
try:
1384+
return await conn.retry.call_with_retry(
1385+
lambda: execute(conn, stack, raise_on_error),
1386+
lambda error: self._disconnect_raise_reset(conn, error),
1387+
)
1388+
except asyncio.CancelledError:
1389+
# not supposed to be possible, yet here we are
1390+
await conn.disconnect(nowait=True)
1391+
raise
1392+
finally:
1393+
await self.reset()
1394+
13561395
async def execute(self, raise_on_error: bool = True):
13571396
"""Execute all the commands in the current pipeline"""
13581397
stack = self.command_stack
@@ -1375,15 +1414,10 @@ async def execute(self, raise_on_error: bool = True):
13751414

13761415
try:
13771416
return await asyncio.shield(
1378-
conn.retry.call_with_retry(
1379-
lambda: execute(conn, stack, raise_on_error),
1380-
lambda error: self._disconnect_raise_reset(conn, error),
1381-
)
1417+
self._try_execute(conn, execute, stack, raise_on_error)
13821418
)
1383-
except asyncio.CancelledError:
1384-
# not supposed to be possible, yet here we are
1385-
await conn.disconnect(nowait=True)
1386-
raise
1419+
except RuntimeError:
1420+
await self.reset()
13871421
finally:
13881422
await self.reset()
13891423

redis/asyncio/cluster.py

+14-7
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,19 @@ async def _parse_and_release(self, connection, *args, **kwargs):
10161016
finally:
10171017
self._free.append(connection)
10181018

1019+
async def _try_parse_response(self, cmd, connection, ret):
1020+
try:
1021+
cmd.result = await asyncio.shield(
1022+
self.parse_response(connection, cmd.args[0], **cmd.kwargs)
1023+
)
1024+
except asyncio.CancelledError:
1025+
await connection.disconnect(nowait=True)
1026+
raise
1027+
except Exception as e:
1028+
cmd.result = e
1029+
ret = True
1030+
return ret
1031+
10191032
async def execute_pipeline(self, commands: List["PipelineCommand"]) -> bool:
10201033
# Acquire connection
10211034
connection = self.acquire_connection()
@@ -1028,13 +1041,7 @@ async def execute_pipeline(self, commands: List["PipelineCommand"]) -> bool:
10281041
# Read responses
10291042
ret = False
10301043
for cmd in commands:
1031-
try:
1032-
cmd.result = await self.parse_response(
1033-
connection, cmd.args[0], **cmd.kwargs
1034-
)
1035-
except Exception as e:
1036-
cmd.result = e
1037-
ret = True
1044+
ret = await asyncio.shield(self._try_parse_response(cmd, connection, ret))
10381045

10391046
# Release connection
10401047
self._free.append(connection)

setup.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
long_description_content_type="text/markdown",
99
keywords=["Redis", "key-value store", "database"],
1010
license="MIT",
11-
version="4.4.3",
11+
version="4.4.4",
1212
packages=find_packages(
1313
include=[
1414
"redis",

tests/test_asyncio/test_cluster.py

-17
Original file line numberDiff line numberDiff line change
@@ -340,23 +340,6 @@ async def test_from_url(self, request: FixtureRequest) -> None:
340340
rc = RedisCluster.from_url("rediss://localhost:16379")
341341
assert rc.connection_kwargs["connection_class"] is SSLConnection
342342

343-
async def test_asynckills(self, r) -> None:
344-
345-
await r.set("foo", "foo")
346-
await r.set("bar", "bar")
347-
348-
t = asyncio.create_task(r.get("foo"))
349-
await asyncio.sleep(1)
350-
t.cancel()
351-
try:
352-
await t
353-
except asyncio.CancelledError:
354-
pytest.fail("connection is left open with unread response")
355-
356-
assert await r.get("bar") == b"bar"
357-
assert await r.ping()
358-
assert await r.get("foo") == b"foo"
359-
360343
async def test_max_connections(
361344
self, create_redis: Callable[..., RedisCluster]
362345
) -> None:

tests/test_asyncio/test_connection.py

-22
Original file line numberDiff line numberDiff line change
@@ -41,28 +41,6 @@ async def test_invalid_response(create_redis):
4141
await r.connection.disconnect()
4242

4343

44-
@pytest.mark.onlynoncluster
45-
async def test_asynckills(create_redis):
46-
47-
for b in [True, False]:
48-
r = await create_redis(single_connection_client=b)
49-
50-
await r.set("foo", "foo")
51-
await r.set("bar", "bar")
52-
53-
t = asyncio.create_task(r.get("foo"))
54-
await asyncio.sleep(1)
55-
t.cancel()
56-
try:
57-
await t
58-
except asyncio.CancelledError:
59-
pytest.fail("connection left open with unread response")
60-
61-
assert await r.get("bar") == b"bar"
62-
assert await r.ping()
63-
assert await r.get("foo") == b"foo"
64-
65-
6644
@skip_if_server_version_lt("4.0.0")
6745
@pytest.mark.redismod
6846
@pytest.mark.onlynoncluster

0 commit comments

Comments
 (0)