fix: keep per-profile models disk cache across /api/profile/switch
POST /api/profile/switch called invalidate_models_cache(), whose test-isolation contract also unlinks models_cache.<profile>.json. On a switch nothing about the sources changed, so the delete only forced a full cold rebuild (live provider fetch_models HTTPS calls, ~490k deepcopy calls) on the very next /api/models — several seconds on every switch. The disk cache is already keyed per profile and rejected on read when _models_cache_source_fingerprint() differs, so it is safe to keep. Add a delete_disk keyword (default True — every existing caller keeps the old behaviour) and have the switch route pass delete_disk=False. Regression tests cover: memory-only drop preserves the disk file, default call still unlinks it, the next get_available_models() serves the disk snapshot without a live rebuild, and the switch route opts out. Two existing zero-arg stubs of invalidate_models_cache accept **kwargs now.
This commit is contained in:
1 parent
173e673c76
commit
7bb12d0aab
5 files changed
+147
-9
No files matched your search
+11
-2
@@ -8156,7 +8156,7 @@ def _get_fresh_memory_models_cache(now: float) -> dict | None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
def invalidate_models_cache():
|
def invalidate_models_cache(*, delete_disk: bool = True):
|
||||||
"""Force the TTL cache for get_available_models() to be cleared.
|
"""Force the TTL cache for get_available_models() to be cleared.
|
||||||
|
|
||||||
Call this after modifying config.cfg in-memory (e.g. in tests) so
|
Call this after modifying config.cfg in-memory (e.g. in tests) so
|
||||||
@@ -8169,6 +8169,14 @@ def invalidate_models_cache():
|
|||||||
that call invalidate_models_cache() still get back the previous test's
|
that call invalidate_models_cache() still get back the previous test's
|
||||||
result from the disk cache because the disk hit is checked before the memory
|
result from the disk cache because the disk hit is checked before the memory
|
||||||
cache rebuild runs.
|
cache rebuild runs.
|
||||||
|
|
||||||
|
``delete_disk=False`` drops only the in-memory snapshot and leaves the
|
||||||
|
per-profile disk cache in place. Use it when the *sources* have not
|
||||||
|
changed and the caller merely needs the next request to re-resolve which
|
||||||
|
profile's catalog to serve (e.g. a per-client profile switch): the disk
|
||||||
|
cache is already keyed per profile and guarded by
|
||||||
|
_models_cache_source_fingerprint(), so a stale snapshot is rejected on
|
||||||
|
read without paying for a full rebuild (live provider fetches).
|
||||||
"""
|
"""
|
||||||
global _cache_build_in_progress, _available_models_cache, _available_models_cache_ts
|
global _cache_build_in_progress, _available_models_cache, _available_models_cache_ts
|
||||||
global _available_models_live_rebuild_ts, _available_models_cache_source_fingerprint, _cache_build_cv
|
global _available_models_live_rebuild_ts, _available_models_cache_source_fingerprint, _cache_build_cv
|
||||||
@@ -8187,7 +8195,8 @@ def invalidate_models_cache():
|
|||||||
_CREDENTIAL_POOL_CACHE.clear()
|
_CREDENTIAL_POOL_CACHE.clear()
|
||||||
# Also delete the disk cache so the next cold build starts fresh.
|
# Also delete the disk cache so the next cold build starts fresh.
|
||||||
# Disk delete is outside the lock — file I/O shouldn't block other readers.
|
# Disk delete is outside the lock — file I/O shouldn't block other readers.
|
||||||
_delete_models_cache_on_disk()
|
if delete_disk:
|
||||||
|
_delete_models_cache_on_disk()
|
||||||
try:
|
try:
|
||||||
from api.plugin_providers import invalidate_plugin_model_provider_cache
|
from api.plugin_providers import invalidate_plugin_model_provider_cache
|
||||||
|
|
||||||
|
|||||||
+6
-3
@@ -16858,11 +16858,14 @@ def handle_post(handler, parsed) -> bool:
|
|||||||
# process_wide=False: don't mutate the process-global _active_profile.
|
# process_wide=False: don't mutate the process-global _active_profile.
|
||||||
# Per-client profile is managed via cookie + thread-local (#798).
|
# Per-client profile is managed via cookie + thread-local (#798).
|
||||||
result = switch_profile(name, process_wide=False)
|
result = switch_profile(name, process_wide=False)
|
||||||
# Invalidate the models cache so the very next /api/models request
|
# Drop the in-memory catalog so the very next /api/models request
|
||||||
# rebuilds from the new profile's config.yaml rather than returning
|
# re-resolves the new profile's config.yaml rather than returning
|
||||||
# the old profile's cached model list (#1200 — profile-switch model bug).
|
# the old profile's cached model list (#1200 — profile-switch model bug).
|
||||||
|
# Keep the per-profile disk cache: it is fingerprint-guarded, and
|
||||||
|
# deleting it forced a full cold rebuild (live provider fetches,
|
||||||
|
# several seconds) on every switch even though nothing changed.
|
||||||
from api.config import invalidate_models_cache
|
from api.config import invalidate_models_cache
|
||||||
invalidate_models_cache()
|
invalidate_models_cache(delete_disk=False)
|
||||||
try:
|
try:
|
||||||
from api.gateway_watcher import restart_watcher_for_profile
|
from api.gateway_watcher import restart_watcher_for_profile
|
||||||
restart_watcher_for_profile(name)
|
restart_watcher_for_profile(name)
|
||||||
|
|||||||
@@ -316,7 +316,7 @@ def test_profile_switch_restarts_watcher_best_effort(monkeypatch):
|
|||||||
monkeypatch.setattr(routes, "read_body", lambda handler: {"name": "demo"})
|
monkeypatch.setattr(routes, "read_body", lambda handler: {"name": "demo"})
|
||||||
monkeypatch.setattr(profiles, "_validate_profile_name", lambda name: None)
|
monkeypatch.setattr(profiles, "_validate_profile_name", lambda name: None)
|
||||||
monkeypatch.setattr(profiles, "switch_profile", lambda name, process_wide=False: {"ok": True, "name": name})
|
monkeypatch.setattr(profiles, "switch_profile", lambda name, process_wide=False: {"ok": True, "name": name})
|
||||||
monkeypatch.setattr(config, "invalidate_models_cache", lambda: calls.append("cache"))
|
monkeypatch.setattr(config, "invalidate_models_cache", lambda *_a, **_kw: calls.append("cache"))
|
||||||
monkeypatch.setattr(gateway_watcher, "restart_watcher_for_profile", lambda name: calls.append(("watcher", name)))
|
monkeypatch.setattr(gateway_watcher, "restart_watcher_for_profile", lambda name: calls.append(("watcher", name)))
|
||||||
|
|
||||||
handler = _FakeHandler()
|
handler = _FakeHandler()
|
||||||
@@ -335,7 +335,7 @@ def test_profile_switch_response_survives_watcher_restart_failure(monkeypatch):
|
|||||||
monkeypatch.setattr(routes, "read_body", lambda handler: {"name": "demo"})
|
monkeypatch.setattr(routes, "read_body", lambda handler: {"name": "demo"})
|
||||||
monkeypatch.setattr(profiles, "_validate_profile_name", lambda name: None)
|
monkeypatch.setattr(profiles, "_validate_profile_name", lambda name: None)
|
||||||
monkeypatch.setattr(profiles, "switch_profile", lambda name, process_wide=False: {"ok": True, "name": name})
|
monkeypatch.setattr(profiles, "switch_profile", lambda name, process_wide=False: {"ok": True, "name": name})
|
||||||
monkeypatch.setattr(config, "invalidate_models_cache", lambda: None)
|
monkeypatch.setattr(config, "invalidate_models_cache", lambda *_a, **_kw: None)
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
gateway_watcher,
|
gateway_watcher,
|
||||||
"restart_watcher_for_profile",
|
"restart_watcher_for_profile",
|
||||||
|
|||||||
@@ -0,0 +1,126 @@
|
|||||||
|
"""Profile switch must not discard the per-profile models disk cache.
|
||||||
|
|
||||||
|
``POST /api/profile/switch`` calls ``invalidate_models_cache()`` so the next
|
||||||
|
``/api/models`` re-resolves the new profile's catalog (#1200). That helper also
|
||||||
|
deleted the on-disk ``models_cache.<profile>.json`` — a test-isolation
|
||||||
|
behaviour — which forced a full cold rebuild (live provider ``fetch_models``
|
||||||
|
HTTPS calls, ~490k deepcopy calls) on every switch even though no source had
|
||||||
|
changed. The disk cache is already keyed per profile and rejected on read when
|
||||||
|
``_models_cache_source_fingerprint()`` differs, so deleting it buys no
|
||||||
|
correctness on the switch path and only costs seconds of latency.
|
||||||
|
|
||||||
|
The switch route now uses ``invalidate_models_cache(delete_disk=False)``.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
REPO = Path(__file__).resolve().parent.parent
|
||||||
|
|
||||||
|
|
||||||
|
def _catalog(label: str) -> dict:
|
||||||
|
return {
|
||||||
|
"active_provider": "openai",
|
||||||
|
"default_model": label,
|
||||||
|
"configured_model_badges": {},
|
||||||
|
"groups": [
|
||||||
|
{
|
||||||
|
"provider": "OpenAI",
|
||||||
|
"provider_id": "openai",
|
||||||
|
"models": [{"id": label, "label": label, "supports_fast_tier": False}],
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"aliases": {},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _prime_memory_cache(monkeypatch, catalog: dict):
|
||||||
|
import api.config as cfg
|
||||||
|
|
||||||
|
monkeypatch.setattr(cfg, "_available_models_cache", catalog, raising=False)
|
||||||
|
monkeypatch.setattr(cfg, "_available_models_cache_ts", 1.0, raising=False)
|
||||||
|
monkeypatch.setattr(cfg, "_available_models_cache_source_fingerprint", {"p": "x"}, raising=False)
|
||||||
|
monkeypatch.setattr(cfg, "_cache_build_in_progress", False, raising=False)
|
||||||
|
|
||||||
|
|
||||||
|
def test_invalidate_keeps_disk_cache_when_delete_disk_false(tmp_path, monkeypatch):
|
||||||
|
import api.config as cfg
|
||||||
|
|
||||||
|
cache_path = tmp_path / "models_cache.profile.json"
|
||||||
|
cache_path.write_text("{}", encoding="utf-8")
|
||||||
|
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||||
|
_prime_memory_cache(monkeypatch, _catalog("old-profile-model"))
|
||||||
|
|
||||||
|
cfg.invalidate_models_cache(delete_disk=False)
|
||||||
|
|
||||||
|
# In-memory snapshot dropped so the next request re-resolves the profile...
|
||||||
|
assert cfg._available_models_cache is None
|
||||||
|
assert cfg._available_models_cache_source_fingerprint is None
|
||||||
|
assert cfg._models_cache_provenance is None
|
||||||
|
# ...but the fingerprint-guarded disk cache survives.
|
||||||
|
assert cache_path.exists(), "delete_disk=False must not unlink the per-profile disk cache"
|
||||||
|
|
||||||
|
|
||||||
|
def test_invalidate_default_still_deletes_disk_cache(tmp_path, monkeypatch):
|
||||||
|
"""Test-isolation contract is unchanged for every existing caller."""
|
||||||
|
import api.config as cfg
|
||||||
|
|
||||||
|
cache_path = tmp_path / "models_cache.profile.json"
|
||||||
|
cache_path.write_text("{}", encoding="utf-8")
|
||||||
|
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||||
|
_prime_memory_cache(monkeypatch, _catalog("old-profile-model"))
|
||||||
|
|
||||||
|
cfg.invalidate_models_cache()
|
||||||
|
|
||||||
|
assert cfg._available_models_cache is None
|
||||||
|
assert not cache_path.exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_memory_only_invalidate_serves_next_request_from_disk_without_rebuild(tmp_path, monkeypatch):
|
||||||
|
"""After a memory-only drop, get_available_models() must reload the disk
|
||||||
|
snapshot instead of running the live rebuild."""
|
||||||
|
import api.config as cfg
|
||||||
|
|
||||||
|
cache_path = tmp_path / "models_cache.profile.json"
|
||||||
|
cache_path.write_text("{}", encoding="utf-8")
|
||||||
|
disk_catalog = _catalog("disk-model")
|
||||||
|
monkeypatch.setattr(cfg, "_get_models_cache_path", lambda: cache_path)
|
||||||
|
monkeypatch.setattr(cfg, "_load_models_cache_from_disk", lambda: disk_catalog if cache_path.exists() else None)
|
||||||
|
monkeypatch.setattr(cfg, "_load_stale_models_cache_from_disk", lambda: None)
|
||||||
|
monkeypatch.setattr(cfg, "_models_cache_source_fingerprint", lambda: {"profile": "demo"})
|
||||||
|
# Pin config mtime tracking so an unrelated config change (or leaked
|
||||||
|
# state from an earlier test) cannot force the reload branch.
|
||||||
|
cfg_file = tmp_path / "config.yaml"
|
||||||
|
cfg_file.write_text("model: {}\n", encoding="utf-8")
|
||||||
|
monkeypatch.setattr(cfg, "_get_config_path", lambda: cfg_file)
|
||||||
|
monkeypatch.setattr(cfg, "_cfg_path", cfg_file, raising=False)
|
||||||
|
monkeypatch.setattr(cfg, "_cfg_mtime", cfg_file.stat().st_mtime, raising=False)
|
||||||
|
|
||||||
|
def _unexpected_rebuild(*_a, **_kw):
|
||||||
|
raise AssertionError("profile switch must not trigger a live models rebuild")
|
||||||
|
|
||||||
|
monkeypatch.setattr(cfg, "_invoke_models_rebuild", _unexpected_rebuild)
|
||||||
|
_prime_memory_cache(monkeypatch, _catalog("stale-memory-model"))
|
||||||
|
|
||||||
|
cfg.invalidate_models_cache(delete_disk=False)
|
||||||
|
result = cfg.get_available_models()
|
||||||
|
|
||||||
|
assert result["default_model"] == "disk-model"
|
||||||
|
|
||||||
|
|
||||||
|
def test_profile_switch_route_preserves_disk_cache():
|
||||||
|
"""The switch handler must opt out of the disk delete."""
|
||||||
|
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||||
|
start = src.index('if parsed.path == "/api/profile/switch":')
|
||||||
|
end = src.index('if parsed.path == "/api/profile/create":', start)
|
||||||
|
block = src[start:end]
|
||||||
|
|
||||||
|
calls = re.findall(r"invalidate_models_cache\(([^)]*)\)", block)
|
||||||
|
assert calls, "switch route must still invalidate the in-memory models cache (#1200)"
|
||||||
|
for args in calls:
|
||||||
|
assert "delete_disk=False" in args, (
|
||||||
|
"/api/profile/switch must call invalidate_models_cache(delete_disk=False); "
|
||||||
|
"deleting the disk cache forces a multi-second cold rebuild on every switch"
|
||||||
|
)
|
||||||
@@ -438,7 +438,7 @@ def test_profile_switch_accepts_bound_profile(monkeypatch):
|
|||||||
monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True)
|
monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True)
|
||||||
monkeypatch.setattr(routes, "read_body", lambda _handler: {"name": "devops"})
|
monkeypatch.setattr(routes, "read_body", lambda _handler: {"name": "devops"})
|
||||||
monkeypatch.setattr("api.profiles.switch_profile", lambda name, process_wide=False: {"ok": True, "profile": name})
|
monkeypatch.setattr("api.profiles.switch_profile", lambda name, process_wide=False: {"ok": True, "profile": name})
|
||||||
monkeypatch.setattr("api.config.invalidate_models_cache", lambda: None)
|
monkeypatch.setattr("api.config.invalidate_models_cache", lambda *_a, **_kw: None)
|
||||||
monkeypatch.setattr("api.gateway_watcher.restart_watcher_for_profile", lambda _name: None)
|
monkeypatch.setattr("api.gateway_watcher.restart_watcher_for_profile", lambda _name: None)
|
||||||
|
|
||||||
routes.handle_post(handler, SimpleNamespace(path="/api/profile/switch", query=""))
|
routes.handle_post(handler, SimpleNamespace(path="/api/profile/switch", query=""))
|
||||||
@@ -851,7 +851,7 @@ def test_consumers_route_through_auth_owner(monkeypatch):
|
|||||||
monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True)
|
monkeypatch.setattr(routes, "_check_csrf", lambda _handler: True)
|
||||||
monkeypatch.setattr(routes, "read_body", lambda _handler: {"name": "devops"})
|
monkeypatch.setattr(routes, "read_body", lambda _handler: {"name": "devops"})
|
||||||
monkeypatch.setattr("api.profiles.switch_profile", lambda name, process_wide=False: {"ok": True, "profile": name})
|
monkeypatch.setattr("api.profiles.switch_profile", lambda name, process_wide=False: {"ok": True, "profile": name})
|
||||||
monkeypatch.setattr("api.config.invalidate_models_cache", lambda: None)
|
monkeypatch.setattr("api.config.invalidate_models_cache", lambda *_a, **_kw: None)
|
||||||
monkeypatch.setattr("api.gateway_watcher.restart_watcher_for_profile", lambda _name: None)
|
monkeypatch.setattr("api.gateway_watcher.restart_watcher_for_profile", lambda _name: None)
|
||||||
|
|
||||||
routes.handle_get(handler, SimpleNamespace(path="/api/auth/status", query=""))
|
routes.handle_get(handler, SimpleNamespace(path="/api/auth/status", query=""))
|
||||||
|
|||||||
Reference in new issue
Block a user