mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-12 06:08:05 +00:00
fix(odds): stop a stalled ESPN taking the whole plugin update with it (#449)
Odds are fetched per live game from inside SportsLive.update(), with
show_odds defaulting on, and the plugin executor kills an operation at
30s. The odds request timeout was also 30s, so a single stalled request
consumed the entire budget and the update carrying every game's score
was killed.
Out of season that is invisible: preseason week 1 returns one game. A
Sunday slate is around sixteen, so the odds of at least one slow request
rise sharply just as the cost of losing the update does.
Shorten the request timeout to 5s, and after a network failure skip the
network for 60s. The timeout alone is not enough -- sixteen consecutive
5s timeouts still blow through -- and when ESPN is unreachable it is
unreachable for the whole slate, so the first failure already answers
the question for the rest of the pass.
before: one stalled request = 30s = the entire budget
after : 5s, the rest of the slate skipped, retry after 60s
The stale-cache fallback is unchanged: the cache is consulted before any
of this, and the failing request still falls back to it.
An earlier version of this branch also jittered the cache TTL to stagger
expiry across a slate. That has been dropped: CacheManager.set() stores
ttl for compatibility but the read path expires entries by a per-type
max_age (1800s for odds), so the jitter was inert. Making the read path
honour a per-entry ttl is a real fix but changes a contract 48 plugin
call sites already rely on, which is not a change to make two weeks
before the season.
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,8 @@ Follows LEDMatrix configuration management patterns:
|
||||
"""
|
||||
|
||||
import logging
|
||||
import time
|
||||
|
||||
import requests
|
||||
import json
|
||||
from typing import Dict, Any, Optional, List
|
||||
@@ -45,7 +47,14 @@ class BaseOddsManager:
|
||||
|
||||
# Configuration with defaults
|
||||
self.update_interval = 3600 # 1 hour default
|
||||
self.request_timeout = 30 # 30 seconds default
|
||||
# Well under the plugin executor's 30s operation budget. At 30s a
|
||||
# single stalled ESPN request consumed the entire budget and the whole
|
||||
# update() was killed -- and odds are fetched per live game, inside the
|
||||
# live update loop, with show_odds defaulting on. Losing one game's
|
||||
# odds beats losing the update that carries every game's score.
|
||||
self.request_timeout = 5
|
||||
# Set when a request fails; until then, skip the network entirely.
|
||||
self._skip_network_until = 0.0
|
||||
self.cache_ttl = 1800 # 30 minutes default
|
||||
|
||||
# Load configuration if available
|
||||
@@ -73,6 +82,14 @@ class BaseOddsManager:
|
||||
except Exception as e:
|
||||
self.logger.warning(f"Failed to load BaseOddsManager configuration: {e}")
|
||||
|
||||
# After a network failure, stop trying for this long and serve cache only.
|
||||
# A short per-request timeout bounds one stall, but a full Sunday slate is
|
||||
# ~16 games fetched in a loop, so 16 consecutive timeouts still blow the
|
||||
# budget. When ESPN is unreachable it is unreachable for all of them, so
|
||||
# the first failure is enough to know: skip the rest of this pass and try
|
||||
# again shortly.
|
||||
_FAILURE_COOLDOWN = 60.0
|
||||
|
||||
def get_odds(self, sport: str | None, league: str | None, event_id: str,
|
||||
update_interval_seconds: int = None) -> Optional[Dict[str, Any]]:
|
||||
"""
|
||||
@@ -101,6 +118,16 @@ class BaseOddsManager:
|
||||
self.logger.info(f"Using cached odds from ESPN for {cache_key}")
|
||||
return cached_data
|
||||
|
||||
if time.monotonic() < self._skip_network_until:
|
||||
# A recent request failed, so ESPN is very likely still unreachable.
|
||||
# Returning now keeps the caller's update inside its time budget
|
||||
# instead of paying the timeout again for every remaining game.
|
||||
self.logger.debug(
|
||||
"Skipping odds fetch for %s: a recent request failed, holding off "
|
||||
"for another %.0fs", cache_key,
|
||||
self._skip_network_until - time.monotonic())
|
||||
return None
|
||||
|
||||
self.logger.info(f"Cache miss - fetching fresh odds from ESPN for {cache_key}")
|
||||
|
||||
try:
|
||||
@@ -121,6 +148,8 @@ class BaseOddsManager:
|
||||
response.raise_for_status()
|
||||
raw_data = response.json()
|
||||
|
||||
self._skip_network_until = 0.0 # reachable again
|
||||
|
||||
self.logger.debug(f"Received raw odds data from ESPN: {json.dumps(raw_data, indent=2)}")
|
||||
|
||||
odds_data = self._extract_espn_data(raw_data)
|
||||
@@ -140,7 +169,11 @@ class BaseOddsManager:
|
||||
return odds_data
|
||||
|
||||
except requests.exceptions.RequestException as e:
|
||||
self.logger.error(f"Error fetching odds from ESPN API for {cache_key}: {e}")
|
||||
self._skip_network_until = time.monotonic() + self._FAILURE_COOLDOWN
|
||||
self.logger.error(
|
||||
"Error fetching odds from ESPN API for %s: %s. Holding off on odds "
|
||||
"for %.0fs so a slate of games does not pay this timeout each.",
|
||||
cache_key, e, self._FAILURE_COOLDOWN)
|
||||
except json.JSONDecodeError:
|
||||
self.logger.error(f"Error decoding JSON response from ESPN API for {cache_key}.")
|
||||
|
||||
|
||||
@@ -87,7 +87,11 @@ class TestGetOdds:
|
||||
assert '/events/401/competitions/401/odds' in url
|
||||
assert url == ('https://sports.core.api.espn.com/v2/sports/football/'
|
||||
'leagues/nfl/events/401/competitions/401/odds')
|
||||
assert mock_get.call_args.kwargs['timeout'] == 30
|
||||
# The number matters less than the property: a single stalled request
|
||||
# must not be able to consume the plugin executor's 30s operation
|
||||
# budget, since odds are fetched per live game inside update().
|
||||
assert mock_get.call_args.kwargs['timeout'] == 5
|
||||
assert mock_get.call_args.kwargs['timeout'] < 30
|
||||
|
||||
def test_ncaa_fb_maps_to_college_football(self, manager, mock_get):
|
||||
manager.get_odds('football', 'ncaa_fb', '401')
|
||||
@@ -355,5 +359,5 @@ class TestLoadConfiguration:
|
||||
manager = BaseOddsManager(cache_manager, config_manager=config_manager)
|
||||
|
||||
assert manager.update_interval == 3600
|
||||
assert manager.request_timeout == 30
|
||||
assert manager.request_timeout == 5
|
||||
assert manager.cache_ttl == 1800
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
"""Tests that a slow ESPN cannot take a whole plugin update with it.
|
||||
|
||||
Odds are fetched per live game from inside SportsLive.update(), with show_odds
|
||||
defaulting on, and the plugin executor kills an operation at 30s. The odds
|
||||
request timeout was also 30s, so one stalled request consumed the entire budget
|
||||
and the update carrying every game's score was killed:
|
||||
|
||||
00:43:43 ERROR plugin football-scoreboard operation timed out after 30.0s
|
||||
01:43:43 ERROR plugin football-scoreboard operation timed out after 30.0s
|
||||
|
||||
Invisible out of season -- preseason week 1 returns a single game -- and a
|
||||
Sunday slate is around sixteen.
|
||||
"""
|
||||
|
||||
from unittest.mock import Mock
|
||||
|
||||
from src.base_odds_manager import BaseOddsManager
|
||||
|
||||
PLUGIN_BUDGET = 30.0 # PluginExecutor(default_timeout=30.0)
|
||||
|
||||
|
||||
def _manager(cache=None):
|
||||
cache = cache or Mock()
|
||||
cache.get_with_auto_strategy.return_value = None
|
||||
return BaseOddsManager(cache_manager=cache, config_manager=None)
|
||||
|
||||
|
||||
class TestRequestTimeout:
|
||||
def test_leaves_room_in_the_operation_budget(self):
|
||||
assert _manager().request_timeout < PLUGIN_BUDGET / 2
|
||||
|
||||
def test_the_timeout_is_the_one_actually_used(self):
|
||||
m = _manager()
|
||||
import src.base_odds_manager as mod
|
||||
real = mod.requests.get
|
||||
try:
|
||||
mod.requests.get = Mock(side_effect=mod.requests.exceptions.Timeout("x"))
|
||||
m.get_odds("football", "nfl", "401")
|
||||
assert mod.requests.get.call_args.kwargs["timeout"] == m.request_timeout
|
||||
finally:
|
||||
mod.requests.get = real
|
||||
|
||||
|
||||
class TestSlowEspnCannotKillTheUpdate:
|
||||
def test_one_failure_stops_the_rest_of_the_slate_hitting_the_network(self):
|
||||
m = _manager()
|
||||
import src.base_odds_manager as mod
|
||||
real = mod.requests.get
|
||||
calls = {"n": 0}
|
||||
|
||||
def timeout(*a, **k):
|
||||
calls["n"] += 1
|
||||
raise mod.requests.exceptions.Timeout("timed out")
|
||||
|
||||
try:
|
||||
mod.requests.get = timeout
|
||||
for i in range(16): # a full slate, one game at a time
|
||||
m.get_odds("football", "nfl", "4018730%02d" % i)
|
||||
finally:
|
||||
mod.requests.get = real
|
||||
|
||||
assert calls["n"] == 1, (
|
||||
"%d games each paid the timeout; the breaker should have stopped "
|
||||
"after the first" % calls["n"])
|
||||
|
||||
def test_worst_case_slate_stays_inside_the_budget(self):
|
||||
m = _manager()
|
||||
assert m.request_timeout * 1 < PLUGIN_BUDGET
|
||||
|
||||
def test_recovery_is_automatic(self):
|
||||
m = _manager()
|
||||
import src.base_odds_manager as mod
|
||||
real_get, real_monotonic = mod.requests.get, mod.time.monotonic
|
||||
clock = {"t": 1000.0}
|
||||
try:
|
||||
mod.time.monotonic = lambda: clock["t"]
|
||||
mod.requests.get = Mock(
|
||||
side_effect=mod.requests.exceptions.Timeout("timed out"))
|
||||
m.get_odds("football", "nfl", "401")
|
||||
assert m._skip_network_until > clock["t"], "breaker did not open"
|
||||
|
||||
clock["t"] += 1
|
||||
before = mod.requests.get.call_count
|
||||
m.get_odds("football", "nfl", "402")
|
||||
assert mod.requests.get.call_count == before, "should not have retried"
|
||||
|
||||
clock["t"] += m._FAILURE_COOLDOWN
|
||||
m.get_odds("football", "nfl", "403")
|
||||
assert mod.requests.get.call_count > before, "never retried"
|
||||
finally:
|
||||
mod.requests.get, mod.time.monotonic = real_get, real_monotonic
|
||||
|
||||
def test_a_healthy_fetch_clears_the_breaker(self):
|
||||
m = _manager()
|
||||
m._skip_network_until = 0.0
|
||||
m._extract_espn_data = Mock(return_value=None)
|
||||
import src.base_odds_manager as mod
|
||||
real = mod.requests.get
|
||||
try:
|
||||
resp = Mock()
|
||||
resp.json.return_value = {}
|
||||
resp.raise_for_status.return_value = None
|
||||
mod.requests.get = Mock(return_value=resp)
|
||||
m.get_odds("football", "nfl", "401")
|
||||
finally:
|
||||
mod.requests.get = real
|
||||
assert m._skip_network_until == 0.0
|
||||
|
||||
def test_the_stale_cache_fallback_still_works(self):
|
||||
# The failing request must still hand back whatever was cached; only
|
||||
# the *subsequent* games skip the network.
|
||||
cache = Mock()
|
||||
cache.get_with_auto_strategy.side_effect = [None, {"details": "stale"}]
|
||||
m = BaseOddsManager(cache_manager=cache, config_manager=None)
|
||||
import src.base_odds_manager as mod
|
||||
real = mod.requests.get
|
||||
try:
|
||||
mod.requests.get = Mock(
|
||||
side_effect=mod.requests.exceptions.Timeout("timed out"))
|
||||
assert m.get_odds("football", "nfl", "401") == {"details": "stale"}
|
||||
finally:
|
||||
mod.requests.get = real
|
||||
Reference in New Issue
Block a user