mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-10 08:28:25 +00:00
fix(redis): unwrap decorated __init__s when deriving the from_url kwargs allowlist
redis-py >= 7.4 decorates AbstractConnection.__init__ with @deprecated_args, whose wrapper is declared (self, *args, **kwargs). _init_arg_names introspects the wrapper directly, so from redis-py 7.4 the MRO walk loses every real connection parameter and the from_url allowlist silently drops socket_timeout and socket_connect_timeout again - the exact regression the allowlist rework fixed, reintroduced one dependency version later. A url-configured Redis that blackholes packets then blocks callers indefinitely instead of timing out. Follow the __wrapped__ chain with inspect.unwrap before introspecting; a no-op for undecorated __init__s. Measured across redis-py lines (socket_timeout present in the allowlist): 6.4.0 before/after: yes/yes. 7.1.0: yes/yes. 7.4.1: NO/yes. 8.1.0: NO/yes. tests/test_litellm/test_redis.py at redis-py 8.1.0: 10 failures before, 3 after (the residual trio is sentinel/cluster password handling, failing identically without this change). Two tests: a decorated-fake proving the unwrap mechanism, and a live-invariant assertion that the installed redis-py's allowlist carries the socket timeouts - the first thing to go red if a future redis-py changes signature declaration again.
This commit is contained in:
parent
0e9cd9893e
commit
99716c5d4b
2 changed files with 55 additions and 1 deletions
|
|
@ -67,12 +67,20 @@ def _init_arg_names(cls: type) -> frozenset[str]:
|
|||
|
||||
Keyword-only parameters are included, and the MRO is walked because redis-py splits a
|
||||
connection's parameters between ``AbstractConnection`` and its concrete subclasses.
|
||||
|
||||
Each ``__init__`` is unwrapped before introspection: redis-py >= 7.4 decorates
|
||||
``AbstractConnection.__init__`` with ``@deprecated_args``, whose wrapper is declared
|
||||
``(self, *args, **kwargs)`` — introspecting the wrapper directly loses every real
|
||||
parameter (``socket_timeout`` included), which silently emptied this allowlist and
|
||||
dropped the socket timeouts from url-configured connections. ``inspect.unwrap``
|
||||
follows the ``__wrapped__`` chain to the true signature and is a no-op on
|
||||
undecorated ``__init__``s.
|
||||
"""
|
||||
return frozenset(
|
||||
name
|
||||
for klass in inspect.getmro(cls)
|
||||
if klass is not object
|
||||
for spec in (inspect.getfullargspec(klass.__init__),)
|
||||
for spec in (inspect.getfullargspec(inspect.unwrap(klass.__init__)),)
|
||||
for name in spec.args + spec.kwonlyargs
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -864,3 +864,49 @@ def test_redis_uses_the_hiredis_response_parser():
|
|||
client = get_redis_client(host="redis-host", port=6379)
|
||||
connection = client.connection_pool.make_connection()
|
||||
assert isinstance(connection._parser, _HiredisParser)
|
||||
|
||||
|
||||
def test_init_arg_names_sees_through_decorated_inits():
|
||||
"""redis-py >= 7.4 wraps AbstractConnection.__init__ with @deprecated_args, whose
|
||||
wrapper is declared (self, *args, **kwargs). Introspecting the wrapper directly
|
||||
yields no real parameters, which silently emptied the from_url allowlist and
|
||||
dropped socket_timeout from url-configured connections. The MRO walk must follow
|
||||
__wrapped__ to the true signature.
|
||||
"""
|
||||
import functools
|
||||
|
||||
from litellm._redis import _init_arg_names
|
||||
|
||||
def deprecating(fn):
|
||||
@functools.wraps(fn)
|
||||
def wrapper(self, *args, **kwargs):
|
||||
return fn(self, *args, **kwargs)
|
||||
|
||||
return wrapper
|
||||
|
||||
class Base:
|
||||
@deprecating
|
||||
def __init__(self, socket_timeout=None, socket_connect_timeout=None):
|
||||
pass
|
||||
|
||||
class Concrete(Base):
|
||||
def __init__(self, host=None, **kwargs):
|
||||
super().__init__(**kwargs)
|
||||
|
||||
names = _init_arg_names(Concrete)
|
||||
assert "socket_timeout" in names
|
||||
assert "socket_connect_timeout" in names
|
||||
assert "host" in names
|
||||
|
||||
|
||||
def test_url_allowlist_always_carries_socket_timeouts():
|
||||
"""The load-bearing invariant behind test_url_config_* against the INSTALLED
|
||||
redis-py, whatever its version: if a redis-py release changes how its __init__
|
||||
signatures are declared (7.4 did, via @deprecated_args), this is the first
|
||||
assertion that goes red.
|
||||
"""
|
||||
from litellm._redis import _get_redis_url_kwargs
|
||||
|
||||
allowed = _get_redis_url_kwargs()
|
||||
assert "socket_timeout" in allowed
|
||||
assert "socket_connect_timeout" in allowed
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue