diff --git a/litellm/caching/redis_cache.py b/litellm/caching/redis_cache.py index 9a670da9100..3dd2b5e9e23 100644 --- a/litellm/caching/redis_cache.py +++ b/litellm/caching/redis_cache.py @@ -85,6 +85,10 @@ def _get_call_stack_info(num_frames: int = 2) -> str: class RedisCache(BaseCache): # if users don't provider one, use the default litellm cache + + # Default Redis major version to assume when version cannot be determined + # Using 7 as it's the modern version that supports LPOP with count parameter + DEFAULT_REDIS_MAJOR_VERSION = 7 def __init__( self, @@ -207,6 +211,35 @@ class RedisCache(BaseCache): return key + def _parse_redis_major_version(self) -> int: + """ + Parse Redis version to extract the major version number. + + Handles multiple version formats: + - Strings: "7.0.0", "6", "7.0.0-rc1", " 7.0.0 " + - Floats: 7.0 (e.g., from AWS ElastiCache Valkey) + - Integers: 7 + - Malformed: "latest", "", "Unknown" (defaults to DEFAULT_REDIS_MAJOR_VERSION) + + Returns: + int: The major version number (defaults to DEFAULT_REDIS_MAJOR_VERSION if unparseable) + """ + if self.redis_version == "Unknown": + return self.DEFAULT_REDIS_MAJOR_VERSION + + try: + version_str = str(self.redis_version).strip() + # Handle cases where there's no dot (e.g., "7" or 7) + if "." in version_str: + major_version = int(version_str.split(".")[0]) + else: + # Direct integer or single-digit string + major_version = int(float(version_str)) + return major_version + except (ValueError, AttributeError): + # Fallback for unparseable versions (e.g., "v7.0.0", "latest") + return self.DEFAULT_REDIS_MAJOR_VERSION + def set_cache(self, key, value, **kwargs): ttl = self.get_ttl(**kwargs) print_verbose( @@ -1259,11 +1292,7 @@ class RedisCache(BaseCache): start_time = time.time() print_verbose(f"LPOP from Redis list: key: {key}, count: {count}") try: - major_version: int = 7 - # Check Redis version and use appropriate method - if self.redis_version != "Unknown": - # Parse version string like "6.0.0" to get major version - major_version = int(self.redis_version.split(".")[0]) + major_version = self._parse_redis_major_version() if count is not None and major_version < 7: # For Redis < 7.0, use pipeline to execute multiple LPOP commands diff --git a/tests/test_litellm/caching/test_redis_cache.py b/tests/test_litellm/caching/test_redis_cache.py index 654645548ff..f3e7953b8ae 100644 --- a/tests/test_litellm/caching/test_redis_cache.py +++ b/tests/test_litellm/caching/test_redis_cache.py @@ -120,3 +120,82 @@ async def test_handle_lpop_count_for_older_redis_versions(monkeypatch): assert result == [b"value1", b"value2"] assert mock_pipeline.lpop.call_count == 2 assert mock_pipeline.execute.call_count == 2 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "redis_version", + [ + # Standard cases + "7.0.0", # Standard Redis string version + 7.0, # Valkey/ElastiCache float version (THE BUG this fix addresses) + 7, # Integer version (e.g., from some Redis forks) + + # Version < 7 + "6", # String without dots, version < 7 + + # Malformed versions (fallback to 7) + "latest", # Non-numeric version + "", # Empty string + -7.0, # Negative float + + # Format variations + " 7.0.0 ", # Whitespace (should be stripped) + "7.0.0-rc1", # Version with suffix + "10.0.0", # Double digit major version + ], +) +async def test_async_lpop_with_float_redis_version( + monkeypatch, redis_no_ping, redis_version +): + """ + Test async_lpop with various Redis version formats (especially float). + + This test specifically addresses the issue where AWS ElastiCache Valkey + returns redis_version as a float (e.g., 7.0) instead of a string (e.g., "7.0.0"), + which caused a 'float' object has no attribute 'split' error when trying to + use the Redis transaction buffer feature. + + The fix converts the version to a string and handles edge cases like: + - Floats (7.0) and integers (7) + - Strings with/without dots ("7" vs "7.0.0") + - Malformed versions ("v7.0.0", "latest") - fallback to version 7 + - Whitespace (" 7.0.0 ") + - Negative versions (fallback to version 7) + + Related: Database deadlock issues when use_redis_transaction_buffer is enabled. + """ + monkeypatch.setenv("REDIS_HOST", "https://my-test-host") + + # Create RedisCache instance + redis_cache = RedisCache() + redis_cache.redis_version = redis_version # Set the version to test + + # Create an AsyncMock for the Redis client + mock_redis_instance = AsyncMock() + mock_redis_instance.__aenter__.return_value = mock_redis_instance + mock_redis_instance.__aexit__.return_value = None + + # Mock lpop to return a test value (Redis >= 7.0 behavior) + mock_redis_instance.lpop.return_value = [b"value1", b"value2"] + + # Mock pipeline for Redis < 7.0 (used when major_version < 7) + mock_pipeline = MagicMock() + mock_pipeline.__aenter__ = AsyncMock(return_value=mock_pipeline) + mock_pipeline.__aexit__ = AsyncMock(return_value=None) + # Make pipeline() a regular method (not async) that returns the mock + mock_redis_instance.pipeline = MagicMock(return_value=mock_pipeline) + + # Mock handle_lpop_count_for_older_redis_versions for Redis < 7 + with patch.object( + redis_cache, "handle_lpop_count_for_older_redis_versions", + return_value=[b"value1", b"value2"] + ): + with patch.object( + redis_cache, "init_async_client", return_value=mock_redis_instance + ): + # Call async_lpop with count - this should not raise AttributeError + result = await redis_cache.async_lpop(key="test_key", count=2) + + # Verify the method completed without error + assert result is not None