Skip to content

Commit a044120

Browse files
committed
[Feat]: Fix env var precedence and add unit tests for RESP env vars
Change precedence so config/CLI args override env vars (env vars serve as defaults). Add unit tests for the precedence logic in both MP and non-MP modes. Signed-off-by: Samuel Shen <slshen@tensormesh.ai>
1 parent 3ed2b13 commit a044120

4 files changed

Lines changed: 212 additions & 37 deletions

File tree

docs/source/kv_cache/storage_backends/resp.rst

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -191,18 +191,19 @@ appearing in logged configuration at startup.
191191
* - Variable
192192
- Description
193193
* - ``LMCACHE_RESP_USERNAME``
194-
- Redis ACL username. Overrides ``username`` in config/JSON.
194+
- Redis ACL username. Used as default when ``username`` is not set in config/JSON.
195195
* - ``LMCACHE_RESP_PASSWORD``
196-
- Redis AUTH password. Overrides ``password`` in config/JSON.
196+
- Redis AUTH password. Used as default when ``password`` is not set in config/JSON.
197197
* - ``LMCACHE_RESP_HOST``
198-
- Redis hostname or IP. Overrides ``host`` in config/JSON/URL.
198+
- Redis hostname or IP. Used as default when ``host`` is not set in config/JSON/URL.
199199
* - ``LMCACHE_RESP_PORT``
200-
- Redis port. Overrides ``port`` in config/JSON/URL.
200+
- Redis port. Used as default when ``port`` is not set in config/JSON/URL.
201201

202-
Environment variables take precedence over values specified in config files
203-
(non-MP) or ``--l2-adapter`` JSON (MP). They are read at adapter creation time
204-
inside the adapter itself, so they are **never stored in the config object** and
205-
**never printed in startup logs**.
202+
Config files (non-MP) and ``--l2-adapter`` JSON (MP) take precedence over
203+
environment variables. Environment variables serve as defaults — they are used
204+
when the corresponding config value is empty or unset. They are read at adapter
205+
creation time inside the adapter itself, so they are **never stored in the
206+
config object** and **never printed in startup logs**.
206207

207208
**Example — MP mode with env vars:**
208209

@@ -338,11 +339,11 @@ The ``--l2-adapter`` JSON accepts these fields:
338339
* - ``username``
339340
- str
340341
- ``""``
341-
- Redis ACL username (leave empty for no auth). Overridden by ``LMCACHE_RESP_USERNAME`` env var.
342+
- Redis ACL username (leave empty for no auth). Falls back to ``LMCACHE_RESP_USERNAME`` env var if empty.
342343
* - ``password``
343344
- str
344345
- ``""``
345-
- Redis AUTH password (leave empty for no auth). Overridden by ``LMCACHE_RESP_PASSWORD`` env var.
346+
- Redis AUTH password (leave empty for no auth). Falls back to ``LMCACHE_RESP_PASSWORD`` env var if empty.
346347
* - ``max_capacity_gb``
347348
- float
348349
- 0

lmcache/v1/distributed/l2_adapters/resp_l2_adapter.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,13 @@ def help(cls) -> str:
110110
"- max_capacity_gb (float): max L2 capacity "
111111
"in GB for usage tracking / eviction "
112112
"(default 0 = disabled)\n\n"
113-
"Environment variable overrides (read at "
114-
"adapter creation, not stored in config):\n"
115-
"- LMCACHE_RESP_USERNAME: overrides username\n"
116-
"- LMCACHE_RESP_PASSWORD: overrides password\n"
117-
"- LMCACHE_RESP_HOST: overrides host\n"
118-
"- LMCACHE_RESP_PORT: overrides port"
113+
"Environment variable defaults (used when "
114+
"config value is empty, read at adapter "
115+
"creation, not stored in config):\n"
116+
"- LMCACHE_RESP_USERNAME: default username\n"
117+
"- LMCACHE_RESP_PASSWORD: default password\n"
118+
"- LMCACHE_RESP_HOST: default host\n"
119+
"- LMCACHE_RESP_PORT: default port"
119120
)
120121

121122

@@ -144,14 +145,13 @@ def _create_resp_l2_adapter(
144145

145146
assert isinstance(config, RESPL2AdapterConfig)
146147

147-
# Environment variables override config values so that
148-
# sensitive credentials are never stored in (or logged
149-
# from) the config object.
150-
host = os.environ.get("LMCACHE_RESP_HOST", "") or config.host
151-
port_str = os.environ.get("LMCACHE_RESP_PORT", "")
152-
port = int(port_str) if port_str else config.port
153-
username = os.environ.get("LMCACHE_RESP_USERNAME", "") or config.username
154-
password = os.environ.get("LMCACHE_RESP_PASSWORD", "") or config.password
148+
# Config/CLI args take precedence over environment variables,
149+
# which serve as defaults. This keeps secrets out of logged
150+
# config while allowing explicit CLI overrides.
151+
host = config.host or os.environ.get("LMCACHE_RESP_HOST", "")
152+
port = config.port if config.port else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
153+
username = config.username or os.environ.get("LMCACHE_RESP_USERNAME", "")
154+
password = config.password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
155155

156156
native_client = LMCacheRedisClient(
157157
host,

lmcache/v1/storage_backend/connector/redis_adapter.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,22 +43,23 @@ def create_connector(self, context: ConnectorContext) -> RemoteConnector:
4343
# Get number of threads for RESP connection pool (default is 8)
4444
self.resp_num_threads = int(extra_config.get("resp_num_threads", 8))
4545

46-
# Get authentication credentials — environment variables
47-
# take precedence over config so that secrets are never
48-
# stored in (or logged from) the config object.
49-
username = os.environ.get("LMCACHE_RESP_USERNAME", "") or str(
50-
extra_config.get("username", "")
51-
)
52-
password = os.environ.get("LMCACHE_RESP_PASSWORD", "") or str(
53-
extra_config.get("password", "")
54-
)
46+
# Config/CLI args take precedence over environment variables,
47+
# which serve as defaults. This keeps secrets out of logged
48+
# config while allowing explicit overrides.
49+
cfg_username = str(extra_config.get("username", ""))
50+
cfg_password = str(extra_config.get("password", ""))
51+
username = cfg_username or os.environ.get("LMCACHE_RESP_USERNAME", "")
52+
password = cfg_password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
5553

5654
parsed_url = parse_remote_url(context.url)
5755

58-
# Allow env vars to override host/port from URL
59-
host = os.environ.get("LMCACHE_RESP_HOST", "") or parsed_url.host
60-
port_str = os.environ.get("LMCACHE_RESP_PORT", "")
61-
port = int(port_str) if port_str else parsed_url.port
56+
# Config/URL values take precedence; env vars are fallback
57+
host = parsed_url.host or os.environ.get("LMCACHE_RESP_HOST", "")
58+
port = (
59+
parsed_url.port
60+
if parsed_url.port
61+
else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
62+
)
6263

6364
logger.info("Creating RESP connector for %s:%d", host, port)
6465
return RESPConnector(
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
"""
3+
Unit tests for RESP adapter environment variable support.
4+
5+
These tests verify the precedence logic (config > env var > default)
6+
without requiring a live Redis server.
7+
"""
8+
9+
# Standard
10+
import os
11+
12+
13+
class TestRESPL2AdapterEnvVars:
14+
"""Test env var resolution in the MP-mode RESP L2 adapter factory."""
15+
16+
def test_env_vars_used_when_config_empty(self, monkeypatch):
17+
"""Env vars should be used when config values are empty/default."""
18+
# First Party
19+
from lmcache.v1.distributed.l2_adapters.resp_l2_adapter import (
20+
RESPL2AdapterConfig,
21+
)
22+
23+
monkeypatch.setenv("LMCACHE_RESP_HOST", "env-host")
24+
monkeypatch.setenv("LMCACHE_RESP_PORT", "7777")
25+
monkeypatch.setenv("LMCACHE_RESP_USERNAME", "env-user")
26+
monkeypatch.setenv("LMCACHE_RESP_PASSWORD", "env-pass")
27+
28+
config = RESPL2AdapterConfig(host="", port=0, username="", password="")
29+
30+
host = config.host or os.environ.get("LMCACHE_RESP_HOST", "")
31+
port = (
32+
config.port
33+
if config.port
34+
else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
35+
)
36+
username = config.username or os.environ.get("LMCACHE_RESP_USERNAME", "")
37+
password = config.password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
38+
39+
assert host == "env-host"
40+
assert port == 7777
41+
assert username == "env-user"
42+
assert password == "env-pass"
43+
44+
def test_config_overrides_env_vars(self, monkeypatch):
45+
"""Config values should take precedence over env vars."""
46+
# First Party
47+
from lmcache.v1.distributed.l2_adapters.resp_l2_adapter import (
48+
RESPL2AdapterConfig,
49+
)
50+
51+
monkeypatch.setenv("LMCACHE_RESP_HOST", "env-host")
52+
monkeypatch.setenv("LMCACHE_RESP_PORT", "7777")
53+
monkeypatch.setenv("LMCACHE_RESP_USERNAME", "env-user")
54+
monkeypatch.setenv("LMCACHE_RESP_PASSWORD", "env-pass")
55+
56+
config = RESPL2AdapterConfig(
57+
host="cfg-host",
58+
port=8888,
59+
username="cfg-user",
60+
password="cfg-pass",
61+
)
62+
63+
host = config.host or os.environ.get("LMCACHE_RESP_HOST", "")
64+
port = (
65+
config.port
66+
if config.port
67+
else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
68+
)
69+
username = config.username or os.environ.get("LMCACHE_RESP_USERNAME", "")
70+
password = config.password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
71+
72+
assert host == "cfg-host"
73+
assert port == 8888
74+
assert username == "cfg-user"
75+
assert password == "cfg-pass"
76+
77+
def test_defaults_when_no_env_and_no_config(self, monkeypatch):
78+
"""Without env vars or config, values should be empty/zero."""
79+
# First Party
80+
from lmcache.v1.distributed.l2_adapters.resp_l2_adapter import (
81+
RESPL2AdapterConfig,
82+
)
83+
84+
monkeypatch.delenv("LMCACHE_RESP_HOST", raising=False)
85+
monkeypatch.delenv("LMCACHE_RESP_PORT", raising=False)
86+
monkeypatch.delenv("LMCACHE_RESP_USERNAME", raising=False)
87+
monkeypatch.delenv("LMCACHE_RESP_PASSWORD", raising=False)
88+
89+
config = RESPL2AdapterConfig(host="", port=0, username="", password="")
90+
91+
host = config.host or os.environ.get("LMCACHE_RESP_HOST", "")
92+
port = (
93+
config.port
94+
if config.port
95+
else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
96+
)
97+
username = config.username or os.environ.get("LMCACHE_RESP_USERNAME", "")
98+
password = config.password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
99+
100+
assert host == ""
101+
assert port == 0
102+
assert username == ""
103+
assert password == ""
104+
105+
106+
class TestRESPConnectorAdapterEnvVars:
107+
"""Test env var resolution in the non-MP RESP connector adapter."""
108+
109+
def test_env_vars_used_when_extra_config_empty(self, monkeypatch):
110+
"""Env vars should be used when extra_config has no credentials."""
111+
monkeypatch.setenv("LMCACHE_RESP_USERNAME", "env-user")
112+
monkeypatch.setenv("LMCACHE_RESP_PASSWORD", "env-pass")
113+
114+
extra_config = {}
115+
116+
cfg_username = str(extra_config.get("username", ""))
117+
cfg_password = str(extra_config.get("password", ""))
118+
username = cfg_username or os.environ.get("LMCACHE_RESP_USERNAME", "")
119+
password = cfg_password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
120+
121+
assert username == "env-user"
122+
assert password == "env-pass"
123+
124+
def test_extra_config_overrides_env_vars(self, monkeypatch):
125+
"""extra_config values should take precedence over env vars."""
126+
monkeypatch.setenv("LMCACHE_RESP_USERNAME", "env-user")
127+
monkeypatch.setenv("LMCACHE_RESP_PASSWORD", "env-pass")
128+
129+
extra_config = {"username": "cfg-user", "password": "cfg-pass"}
130+
131+
cfg_username = str(extra_config.get("username", ""))
132+
cfg_password = str(extra_config.get("password", ""))
133+
username = cfg_username or os.environ.get("LMCACHE_RESP_USERNAME", "")
134+
password = cfg_password or os.environ.get("LMCACHE_RESP_PASSWORD", "")
135+
136+
assert username == "cfg-user"
137+
assert password == "cfg-pass"
138+
139+
def test_host_port_env_var_fallback(self, monkeypatch):
140+
"""Host/port env vars should be used when URL values are empty."""
141+
monkeypatch.setenv("LMCACHE_RESP_HOST", "env-host")
142+
monkeypatch.setenv("LMCACHE_RESP_PORT", "9999")
143+
144+
parsed_host = ""
145+
parsed_port = 0
146+
147+
host = parsed_host or os.environ.get("LMCACHE_RESP_HOST", "")
148+
port = (
149+
parsed_port
150+
if parsed_port
151+
else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
152+
)
153+
154+
assert host == "env-host"
155+
assert port == 9999
156+
157+
def test_url_overrides_env_vars(self, monkeypatch):
158+
"""URL-parsed values should take precedence over env vars."""
159+
monkeypatch.setenv("LMCACHE_RESP_HOST", "env-host")
160+
monkeypatch.setenv("LMCACHE_RESP_PORT", "9999")
161+
162+
parsed_host = "url-host"
163+
parsed_port = 6379
164+
165+
host = parsed_host or os.environ.get("LMCACHE_RESP_HOST", "")
166+
port = (
167+
parsed_port
168+
if parsed_port
169+
else int(os.environ.get("LMCACHE_RESP_PORT", "0"))
170+
)
171+
172+
assert host == "url-host"
173+
assert port == 6379

0 commit comments

Comments
 (0)