fix(proxy): load REDIS_* env vars when cache_params has non-connection keys#26576
Conversation
…n keys (BerriAI#26233) The cache_params env-var fallback in ProxyConfig.load_config was gated on `len(cache_params.keys()) == 0`, so any non-empty cache_params (e.g. just `mode: default_off`) silently dropped the Redis env var config and fell back to in-memory cache. This broke multi-pod deployments because spend_counter_cache.redis_cache never got wired up and each pod tracked counters independently. Replace the length check with a "user did not supply connection details" check: only populate REDIS_HOST / REDIS_PORT / REDIS_PASSWORD from the environment when neither `host` nor `url` is present in cache_params. Other cache_params keys (mode, ttl, etc.) no longer suppress the fallback. Adds two regression tests exercising ProxyConfig.load_config: - cache_params with only non-connection keys → env vars load, RedisCache path - cache_params with explicit host → env vars do not overwrite user config Fixes BerriAI#26233
Greptile SummaryThis PR fixes a silent fallback-to-in-memory-cache bug in Confidence Score: 4/5Safe to merge — the fix is minimal, correct, and well-tested; only a minor test-coverage gap remains. P2-only finding (missing No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Fixes the env-var fallback gate from len(cache_params) == 0 to a "no connection keys" check (host/url absent); minimal, targeted change with good inline comment. |
| tests/test_litellm/proxy/test_proxy_server.py | Adds two new mock-only regression tests that correctly exercise the fix; minor gap — no test covers the url connection-key bypass path. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[load_config: cache true] --> B{cache_params provided?}
B -- yes --> C[merge cache_params_in_config]
B -- no --> D[cache_params is empty dict]
C --> E{cache_type is redis or redis-semantic?}
D --> E
E -- no --> J[_init_cache with params as-is]
E -- yes --> F{host not in cache_params AND url not in cache_params?}
F -- true --> G[Load REDIS_HOST / REDIS_PORT / REDIS_PASSWORD from env]
G --> H[Update cache_params with env values]
H --> J
F -- false --> I[Use cache_params connection details as supplied]
I --> J[_init_cache with cache_params]
Reviews (1): Last reviewed commit: "fix(proxy): load REDIS_* env vars when c..." | Re-trigger Greptile
| @pytest.mark.asyncio | ||
| async def test_explicit_cache_params_host_not_overwritten_by_env_vars(monkeypatch): | ||
| """ | ||
| Companion to test_redis_env_vars_loaded_when_cache_params_has_non_connection_keys. | ||
|
|
||
| When `cache_params` explicitly specifies a `host`, the REDIS_* env vars | ||
| must NOT overwrite it. The env-var fallback only applies when the user | ||
| has supplied no connection details. | ||
| """ | ||
| import tempfile | ||
|
|
||
| from litellm.proxy.proxy_server import ProxyConfig | ||
|
|
||
| monkeypatch.setenv("REDIS_HOST", "env-redis.internal") | ||
| monkeypatch.setenv("REDIS_PORT", "6379") | ||
|
|
||
| test_config = { | ||
| "model_list": [], | ||
| "router_settings": {}, | ||
| "litellm_settings": { | ||
| "cache": True, | ||
| "cache_params": { | ||
| "host": "explicit-redis.internal", | ||
| "port": 1234, | ||
| "mode": "default_off", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: | ||
| yaml.dump(test_config, f) | ||
| config_file_path = f.name | ||
|
|
||
| try: | ||
| proxy_config = ProxyConfig() | ||
| with patch.object(ProxyConfig, "_init_cache") as mock_init: | ||
| await proxy_config.load_config( | ||
| router=MagicMock(), config_file_path=config_file_path | ||
| ) | ||
|
|
||
| mock_init.assert_called_once() | ||
| actual_cache_params = mock_init.call_args.kwargs["cache_params"] | ||
|
|
||
| assert actual_cache_params.get("host") == "explicit-redis.internal" | ||
| assert actual_cache_params.get("port") == 1234 | ||
| assert actual_cache_params.get("mode") == "default_off" | ||
| finally: | ||
| os.unlink(config_file_path) |
There was a problem hiding this comment.
url connection-key coverage
The fix gates the env-var fallback on "host" not in cache_params and "url" not in cache_params, but the companion guard test only exercises the host path. A user supplying cache_params: {url: "redis://...", mode: default_off} relies on the url branch to suppress env-var loading, and that path has no test. Consider adding a third test analogous to this one but using url instead of host.
- BerriAI/litellm#26577 fix(proxy): skip redundant tiktoken recount (merge-after-nits) - BerriAI/litellm#26576 fix(proxy): load REDIS_* env vars when cache_params has non-connection keys (merge-as-is) - QwenLM/qwen-code#3667 fix(cli): refresh static header on model switch (merge-after-nits) - google-gemini/gemini-cli#26005 Fix infinite dialog loop and add ESC support in /skills link (merge-after-nits)
Re-opening previously closed PR #26244 against the new OSS staging branch (the original target
litellm_oss_branchwas deleted on 2026-04-27 as part of the maintainer branch rotation).Relevant issues
Fixes #26233
Type
Bug Fix
Problem
In `ProxyConfig.load_config`, the Redis env-var fallback was gated on `len(cache_params.keys()) == 0`. A config like `cache_params: {mode: default_off}` plus `REDIS_HOST` env vars silently fell back to InMemoryCache because `mode` made cache_params non-empty, skipping the env-var branch. Downstream, `spend_counter_cache.redis_cache` stays `None` and per-pod counters diverge — breaks spend tracking in multi-pod deployments.
Fix
Replace the length check with a "no connection details supplied" check:
```python
if (cache_type == "redis" or cache_type == "redis-semantic") and (
"host" not in cache_params and "url" not in cache_params
):
```
Other cache_params keys (`mode`, `ttl`, etc.) no longer suppress the env-var fallback.
Tests
Two new tests in `tests/test_litellm/proxy/test_proxy_server.py`:
Both fail on the base branch without the fix and pass with it.
Pre-Submission checklist