Move the keepalive interval to config.yaml and tighten the schedule guard
Addresses review feedback on #84928. The tick interval was exposed as HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS. AGENTS.md reserves .env for credentials and puts behavioural thresholds in config.yaml, so the knob moves to `nous.keepalive_interval_seconds`, following the existing `vertex:` section's precedent for non-secret provider settings. The env var is dropped rather than bridged: it was never released, so nothing depends on it. Adding a key to a new section is handled by the deep-merge, so no _config_version bump is required. test_keepalive_interval_fits_inside_the_token_lifetime asserted `900 < 899 * 4 - 120`, which is true for any realistic interval and could never fail. It also tested the wrong value: the configured constant is only a ceiling, while the schedule that ships is the derived tick. Replaced with an assertion over the derived tick for each observed lifetime, which does fail if the derivation constants regress -- verified against both TICKS_PER_LIFETIME=1 and MIN_INTERVAL_SECONDS=5000. Also adds coverage for an unreadable config.yaml, which must fall back to the module default rather than take the keepalive thread down. pytest tests/hermes_cli/test_nous_auth_keepalive.py -> 9 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -43,7 +43,7 @@ NOUS_AUTH_KEEPALIVE_MIN_INTERVAL_SECONDS = 60
|
||||
# expiry without making the thread chatty.
|
||||
NOUS_AUTH_KEEPALIVE_TICKS_PER_LIFETIME = 4
|
||||
NOUS_AUTH_KEEPALIVE_INITIAL_DELAY_SECONDS = 60
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_ENV = "HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS"
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY = "keepalive_interval_seconds"
|
||||
|
||||
_keepalive_lock = threading.Lock()
|
||||
_keepalive_stop = threading.Event()
|
||||
@@ -59,12 +59,30 @@ def _timeout_seconds(value: Optional[float]) -> float:
|
||||
return 15.0
|
||||
|
||||
|
||||
def _nous_config() -> dict:
|
||||
"""Return the ``nous:`` section of config.yaml, or {} on any failure.
|
||||
|
||||
Imported lazily: this module is loaded by the gateway and the web server
|
||||
during startup, and the config loader pulls in a wider dependency graph
|
||||
than the keepalive itself needs.
|
||||
"""
|
||||
try:
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
section = load_config().get("nous")
|
||||
return section if isinstance(section, dict) else {}
|
||||
except Exception:
|
||||
return {}
|
||||
|
||||
|
||||
def _interval_seconds(value: Optional[int]) -> int:
|
||||
"""Resolve the keepalive tick interval.
|
||||
|
||||
Explicit argument wins, then the environment override, then the module
|
||||
default. A non-positive result disables the keepalive thread entirely,
|
||||
which is the documented way to turn it off.
|
||||
Explicit argument wins, then ``nous.keepalive_interval_seconds`` in
|
||||
config.yaml, then the module default. This is a behavioural threshold
|
||||
rather than a credential, so it lives in config.yaml and not in .env.
|
||||
A non-positive result disables the keepalive thread entirely, which is
|
||||
the documented way to turn it off.
|
||||
"""
|
||||
if value is not None:
|
||||
try:
|
||||
@@ -72,15 +90,15 @@ def _interval_seconds(value: Optional[int]) -> int:
|
||||
except (TypeError, ValueError):
|
||||
return NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS
|
||||
|
||||
raw = os.getenv(NOUS_AUTH_KEEPALIVE_INTERVAL_ENV)
|
||||
if raw is None or not raw.strip():
|
||||
raw = _nous_config().get(NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY)
|
||||
if raw is None or (isinstance(raw, str) and not raw.strip()):
|
||||
return NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS
|
||||
try:
|
||||
return int(float(raw))
|
||||
except (TypeError, ValueError):
|
||||
logger.warning(
|
||||
"Ignoring invalid %s=%r; using %ds",
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_ENV,
|
||||
"Ignoring invalid nous.%s=%r; using %ds",
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY,
|
||||
raw,
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user