From 8fb2e1d5bc8b72cf4707beca382d54c7c0e6709a Mon Sep 17 00:00:00 2001 From: Kolade Fajimi <107228310+koladefaj@users.noreply.github.com> Date: Tue, 1 Sep 2026 00:43:43 +0100 Subject: [PATCH] test(redis): assert an async-only cluster kwarg every matrix version declares connection_error_retry_attempts is on the async cluster constructor in redis-py 5.x only; 6.0 removed it in favor of retry. The 6.4.0, 7.4.1 and 8.0.1 legs were failing on that missing parameter name rather than on the behavior under test, while the allow-list itself was doing the right thing on all four versions. decode_responses is async-cluster-only on every version the matrix covers, so it stands in for the same property: the sync cluster class takes it through **kwargs and never names it in its signature. Reverting _get_redis_cluster_kwargs to ignore its client argument still fails both tests on 5.3.1 and 8.0.1. test_async_cluster_passes_async_only_kwargs now builds the real async cluster client and reads connection_kwargs off it, so it no longer needs a patched class factory; the constructor does no I/O. The retry-attempts test keeps its patch, since redis-py >= 6 stores no cluster_error_retry_attempts attribute on the built client and the constructor call is the only place the forwarded value shows up. The _get_redis_cluster_kwargs docstring cited the same two parameters as its examples of async-only kwargs, which is what made the test look reasonable; cluster_error_retry_attempts is on both classes and connection_error_retry_attempts is gone from 6.0 on, so it now names decode_responses instead. --- litellm/_redis.py | 9 ++++----- tests/test_litellm/test_redis.py | 25 ++++++++++++------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/litellm/_redis.py b/litellm/_redis.py index 973aa119816..c0bfa5bfd58 100644 --- a/litellm/_redis.py +++ b/litellm/_redis.py @@ -141,12 +141,11 @@ def _get_redis_cluster_kwargs(client: type | None = None): """Config kwargs the target cluster client's constructor actually accepts. Defaults to the sync ``redis.RedisCluster``, but the async cluster client - (``redis.asyncio.cluster.RedisCluster``) accepts several constructor kwargs - the sync class does not (``cluster_error_retry_attempts``, - ``connection_error_retry_attempts``, ``decode_responses``, ...). Introspecting + (``redis.asyncio.cluster.RedisCluster``) declares connection settings such as + ``decode_responses`` on its own constructor, where the sync class takes them + through ``**kwargs`` and so never names them in its signature. Introspecting only the sync class regardless of which client is actually built silently - drops those for every async cluster caller, including any operator-configured - retry bound meant to cap a stuck node's worst-case latency. + drops those for every async cluster caller. """ if client is None: client = redis.RedisCluster diff --git a/tests/test_litellm/test_redis.py b/tests/test_litellm/test_redis.py index f565e83d51d..a96e8541e06 100644 --- a/tests/test_litellm/test_redis.py +++ b/tests/test_litellm/test_redis.py @@ -611,18 +611,20 @@ def test_retry_attempts_in_cluster_kwargs(): assert "cluster_error_retry_attempts" in kwargs -def test_async_only_retry_kwargs_in_cluster_kwargs_when_async_client_requested(): - """connection_error_retry_attempts exists only on the async cluster client's - constructor. Introspecting the sync class regardless of which client is - actually built silently drops it for every async cluster caller.""" +def test_async_only_kwargs_in_cluster_kwargs_when_async_client_requested(): + """decode_responses is on the async cluster client's constructor and not the sync + one, on every redis-py the matrix covers. Introspecting the sync class regardless + of which client is actually built silently drops it for every async cluster caller.""" sync_kwargs = _get_redis_cluster_kwargs() async_kwargs = _get_redis_cluster_kwargs(async_redis.RedisCluster) - assert "connection_error_retry_attempts" not in sync_kwargs - assert "connection_error_retry_attempts" in async_kwargs + assert "decode_responses" not in sync_kwargs + assert "decode_responses" in async_kwargs -@patch("litellm.caching.redis_cluster_node_isolation.get_litellm_async_redis_cluster_class") +@patch( # test-quality-ok: redis-py >= 6 keeps no cluster_error_retry_attempts attribute on the built client, so the constructor call is the only place the value is observable + "litellm.caching.redis_cluster_node_isolation.get_litellm_async_redis_cluster_class" +) def test_async_cluster_forwards_retry_attempts(mock_get_cluster_class): """Regression: cluster_error_retry_attempts must reach the constructed async cluster client. Silently dropping it removes an operator's only lever for @@ -638,19 +640,16 @@ def test_async_cluster_forwards_retry_attempts(mock_get_cluster_class): assert call_kwargs["cluster_error_retry_attempts"] == 2 -@patch("litellm.caching.redis_cluster_node_isolation.get_litellm_async_redis_cluster_class") -def test_async_cluster_passes_async_only_kwargs(mock_get_cluster_class): +def test_async_cluster_passes_async_only_kwargs(): """Regression: decode_responses is an async-cluster-only constructor arg. When the allow-list came from the sync class it was filtered out and values came back as bytes instead of str.""" - mock_cluster_cls = mock_get_cluster_class.return_value - get_redis_async_client( + client = get_redis_async_client( startup_nodes=[{"host": "cluster-node", "port": 6379}], decode_responses=True, ) - call_kwargs = mock_cluster_cls.call_args[1] - assert call_kwargs["decode_responses"] is True + assert client.connection_kwargs["decode_responses"] is True @pytest.mark.parametrize("cluster_client", [redis.RedisCluster, async_redis.RedisCluster], ids=["sync", "async"])