mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-20 06:30:27 +00:00
fix(config): route stale model reads through shared reload gate (#5220)
This commit is contained in:
committed by
nesquena-hermes
parent
6653b7bd2f
commit
affdb40a1c
+15
-10
@@ -440,6 +440,19 @@ def _apply_config_defaults(config_data: dict) -> None:
|
||||
for key, value in _DEFAULT_EXPERIMENTAL_CONFIG.items():
|
||||
experimental.setdefault(key, value)
|
||||
|
||||
|
||||
def reload_config_if_stale() -> None:
|
||||
"""Refresh config.yaml once for concurrent stale read paths."""
|
||||
with _cfg_lock:
|
||||
try:
|
||||
config_path = _get_config_path()
|
||||
current_mtime = config_path.stat().st_mtime
|
||||
except OSError:
|
||||
current_mtime = 0.0
|
||||
cache_stale = current_mtime != _cfg_mtime or _cfg_path != config_path
|
||||
if not _cfg_cache or (cache_stale and not _cfg_has_in_memory_overrides()):
|
||||
_refresh_config_cache(config_path)
|
||||
|
||||
|
||||
def get_config() -> dict:
|
||||
"""Return the cached config dict, loading from disk if needed."""
|
||||
@@ -450,15 +463,7 @@ def get_config() -> dict:
|
||||
current_mtime = 0.0
|
||||
cache_stale = current_mtime != _cfg_mtime or _cfg_path != config_path
|
||||
if not _cfg_cache or (cache_stale and not _cfg_has_in_memory_overrides()):
|
||||
with _cfg_lock:
|
||||
try:
|
||||
config_path = _get_config_path()
|
||||
current_mtime = config_path.stat().st_mtime
|
||||
except OSError:
|
||||
current_mtime = 0.0
|
||||
cache_stale = current_mtime != _cfg_mtime or _cfg_path != config_path
|
||||
if not _cfg_cache or (cache_stale and not _cfg_has_in_memory_overrides()):
|
||||
_refresh_config_cache(config_path)
|
||||
reload_config_if_stale()
|
||||
# When a test (or runtime caller) has rebound ``cfg`` to a different dict
|
||||
# via monkeypatch.setattr(config, "cfg", ...), return that override rather
|
||||
# than the underlying _cfg_cache. Without this branch, get_config() would
|
||||
@@ -5561,7 +5566,7 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa
|
||||
(_current_mtime != _cfg_mtime or _current_path != _cfg_path)
|
||||
and not _cfg_has_in_memory_overrides()
|
||||
):
|
||||
reload_config()
|
||||
reload_config_if_stale()
|
||||
# ── COLD PATH helper ─────────────────────────────────────────────────────
|
||||
# Extracted so it runs inside _available_models_cache_lock (RLock) to
|
||||
# prevent thundering-herd: only one thread rebuilds while others wait.
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
"""Regression coverage for #5220 config reload stampede collapse."""
|
||||
|
||||
import sys
|
||||
import threading
|
||||
import time
|
||||
import types
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
|
||||
|
||||
@@ -22,17 +24,23 @@ def test_stale_readers_collapse_to_single_reload_when_config_is_hot_reload(tmp_p
|
||||
|
||||
calls = {"n": 0}
|
||||
calls_lock = threading.Lock()
|
||||
real_load = config._load_yaml_config_file_raw
|
||||
real_refresh = config._refresh_config_cache
|
||||
|
||||
def _counted_load(path):
|
||||
def _counted_refresh(path=None):
|
||||
with calls_lock:
|
||||
calls["n"] += 1
|
||||
call_no = calls["n"]
|
||||
if call_no == 1:
|
||||
time.sleep(0.1)
|
||||
return real_load(path)
|
||||
return real_refresh(path)
|
||||
|
||||
monkeypatch.setattr(config, "_load_yaml_config_file_raw", _counted_load)
|
||||
def _unexpected_reload():
|
||||
raise AssertionError(
|
||||
"stale model readers should route through reload_config_if_stale() instead of forced reload_config()"
|
||||
)
|
||||
|
||||
monkeypatch.setattr(config, "_refresh_config_cache", _counted_refresh)
|
||||
monkeypatch.setattr(config, "reload_config", _unexpected_reload)
|
||||
|
||||
def _get():
|
||||
cfg = config.get_config()
|
||||
@@ -79,3 +87,72 @@ def test_explicit_reload_config_forces_disk_refresh(tmp_path, monkeypatch):
|
||||
assert calls["n"] == 1, (
|
||||
"explicit reload_config() should refresh from disk even when caller requests it"
|
||||
)
|
||||
|
||||
|
||||
def test_stale_model_readers_collapse_to_single_reload_when_models_are_requested(tmp_path, monkeypatch):
|
||||
import api.config as config
|
||||
|
||||
config_path = tmp_path / "config.yaml"
|
||||
config_path.write_text(
|
||||
"model:\n provider: openai\n default: cold-model\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
|
||||
with config._yaml_file_cache_lock:
|
||||
config._yaml_file_cache.clear()
|
||||
config.invalidate_models_cache()
|
||||
|
||||
fake_pkg = types.ModuleType("hermes_cli")
|
||||
fake_pkg.__path__ = []
|
||||
fake_models = types.ModuleType("hermes_cli.models")
|
||||
fake_models._PROVIDER_ALIASES = {}
|
||||
fake_models.list_available_providers = lambda: []
|
||||
fake_auth = types.ModuleType("hermes_cli.auth")
|
||||
fake_auth.get_auth_status = lambda provider_id: {"logged_in": False, "key_source": ""}
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth)
|
||||
monkeypatch.setattr(config, "_load_models_cache_from_disk", lambda: None)
|
||||
monkeypatch.setattr(config, "_load_stale_models_cache_from_disk", lambda: None)
|
||||
monkeypatch.setattr(config, "_save_models_cache_to_disk", lambda _data: None)
|
||||
monkeypatch.setattr(config, "_models_cache_source_fingerprint", lambda: {"config": "test"})
|
||||
|
||||
config.reload_config()
|
||||
old_mtime = config._cfg_mtime
|
||||
config_path.write_text(
|
||||
"model:\n provider: openai\n default: refreshed-model\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
config._cfg_mtime = old_mtime - 1.0
|
||||
config.invalidate_models_cache()
|
||||
|
||||
calls = {"n": 0}
|
||||
calls_lock = threading.Lock()
|
||||
real_refresh = config._refresh_config_cache
|
||||
|
||||
def _counted_refresh(path=None):
|
||||
with calls_lock:
|
||||
calls["n"] += 1
|
||||
call_no = calls["n"]
|
||||
if call_no == 1:
|
||||
time.sleep(0.1)
|
||||
return real_refresh(path)
|
||||
|
||||
def _unexpected_reload():
|
||||
raise AssertionError(
|
||||
"stale model readers should route through reload_config_if_stale() instead of forced reload_config()"
|
||||
)
|
||||
|
||||
monkeypatch.setattr(config, "_refresh_config_cache", _counted_refresh)
|
||||
monkeypatch.setattr(config, "reload_config", _unexpected_reload)
|
||||
|
||||
with ThreadPoolExecutor(max_workers=6) as executor:
|
||||
results = list(executor.map(lambda _idx: config.get_available_models(), range(6)))
|
||||
|
||||
assert all(result.get("default_model") == "refreshed-model" for result in results), (
|
||||
f"stale model readers should observe refreshed config, got {results!r}"
|
||||
)
|
||||
assert calls["n"] == 1, (
|
||||
f"expected one config refresh for concurrent model reads, got {calls['n']} "
|
||||
"(base would re-enter refresh work per stale reader)"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user