Fix reactive Nous 401s by ticking keepalive inside the token lifetime
Nous Portal access tokens carry a one-hour lifetime, and the keepalive only refreshes once a token is within ACCESS_TOKEN_REFRESH_SKEW_SECONDS (120s) of expiry. The tick interval was 6 hours, so it could only land inside that 2-minute window by coincidence. In practice every hour rolled over untouched and the next inference call paid a 401 plus a re-auth round trip. Observed on a production install: 71 "refreshed Nous runtime credentials after 401, retrying" events across current logs. Drop the default interval to 15 minutes, which gives four ticks per token lifetime and leaves ample margin under the TTL-minus-skew ceiling of 3480s. Add HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS so the interval is tunable without a source edit, matching the existing HERMES_NOUS_TIMEOUT_SECONDS convention. Zero still disables the thread. Callers pass no interval, so the default is what actually shipped; the signature now resolves at call time rather than binding the constant at import.
This commit is contained in:
@@ -19,8 +19,16 @@ from hermes_cli.auth import (
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS = 6 * 60 * 60
|
||||
# Nous Portal access tokens carry a one-hour lifetime, and the refresh only
|
||||
# fires once a token is within ACCESS_TOKEN_REFRESH_SKEW_SECONDS (120s) of
|
||||
# expiry. A tick interval must therefore be comfortably below
|
||||
# 3600 - 120 = 3480s, or the hour rolls over untouched between ticks and the
|
||||
# next inference call pays a 401 plus a re-auth round trip. The previous
|
||||
# 6-hour interval could only ever land inside the 2-minute refresh window by
|
||||
# coincidence, so in practice every hour expired reactively.
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS = 15 * 60
|
||||
NOUS_AUTH_KEEPALIVE_INITIAL_DELAY_SECONDS = 60
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_ENV = "HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS"
|
||||
|
||||
_keepalive_lock = threading.Lock()
|
||||
_keepalive_stop = threading.Event()
|
||||
@@ -36,6 +44,34 @@ def _timeout_seconds(value: Optional[float]) -> float:
|
||||
return 15.0
|
||||
|
||||
|
||||
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.
|
||||
"""
|
||||
if value is not None:
|
||||
try:
|
||||
return int(value)
|
||||
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():
|
||||
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,
|
||||
raw,
|
||||
NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS,
|
||||
)
|
||||
return NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS
|
||||
|
||||
|
||||
def _entry_state(entry: object) -> dict:
|
||||
return {
|
||||
"agent_key": getattr(entry, "agent_key", None),
|
||||
@@ -144,12 +180,13 @@ def _keepalive_loop(
|
||||
|
||||
def start_nous_auth_keepalive(
|
||||
*,
|
||||
interval_seconds: int = NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS,
|
||||
interval_seconds: Optional[int] = None,
|
||||
initial_delay_seconds: int = NOUS_AUTH_KEEPALIVE_INITIAL_DELAY_SECONDS,
|
||||
min_key_ttl_seconds: int = NOUS_INVOKE_JWT_MIN_TTL_SECONDS,
|
||||
timeout_seconds: Optional[float] = None,
|
||||
) -> Optional[threading.Thread]:
|
||||
"""Start the process-wide Nous auth keepalive thread."""
|
||||
interval_seconds = _interval_seconds(interval_seconds)
|
||||
if interval_seconds <= 0:
|
||||
return None
|
||||
|
||||
|
||||
@@ -1,4 +1,44 @@
|
||||
from hermes_cli import nous_auth_keepalive as keepalive
|
||||
from hermes_cli.auth import ACCESS_TOKEN_REFRESH_SKEW_SECONDS
|
||||
|
||||
NOUS_ACCESS_TOKEN_TTL_SECONDS = 3600
|
||||
|
||||
|
||||
def test_keepalive_interval_fits_inside_the_token_lifetime():
|
||||
"""The tick must land before the hour rolls over, or refresh stays reactive.
|
||||
|
||||
A tick at or above TTL - skew can miss the refresh window entirely, which
|
||||
is what made every hour expire into a 401 plus a re-auth round trip.
|
||||
"""
|
||||
assert (
|
||||
keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS
|
||||
< NOUS_ACCESS_TOKEN_TTL_SECONDS - ACCESS_TOKEN_REFRESH_SKEW_SECONDS
|
||||
)
|
||||
|
||||
|
||||
def test_interval_precedence_and_disable(monkeypatch):
|
||||
monkeypatch.delenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, raising=False)
|
||||
assert (
|
||||
keepalive._interval_seconds(None)
|
||||
== keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS
|
||||
)
|
||||
|
||||
monkeypatch.setenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, "600")
|
||||
assert keepalive._interval_seconds(None) == 600
|
||||
# An explicit argument still outranks the environment.
|
||||
assert keepalive._interval_seconds(300) == 300
|
||||
|
||||
# A malformed override falls back to the default rather than disabling.
|
||||
monkeypatch.setenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, "not-a-number")
|
||||
assert (
|
||||
keepalive._interval_seconds(None)
|
||||
== keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS
|
||||
)
|
||||
|
||||
# Zero remains the documented way to turn the keepalive off.
|
||||
monkeypatch.setenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, "0")
|
||||
assert keepalive._interval_seconds(None) == 0
|
||||
assert keepalive.start_nous_auth_keepalive() is None
|
||||
|
||||
|
||||
def test_keepalive_refreshes_stale_pool_entry(monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user