mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-14 09:03:16 +00:00
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.
This commit is contained in:
parent
c6ff054b26
commit
8fb2e1d5bc
2 changed files with 16 additions and 18 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue