All scheduling now runs through a dispatcher #421

Merged

johnsturgeon merged 1 commit from phase-4-rewire-the-scheduler-onto-the-week-table-410 into main

2026-08-24 19:44:33 +02:00

johnsturgeon commented

2026-08-24 17:28:18 +02:00

Owner

Registration is compare-then-replace rather than unconditional re-add:
add_job(replace_existing=True) recomputes next_run_time as now + interval, so
a per-minute dispatcher would starve an hourly job forever.

Notable pieces:

Score polling is its own job (sync_scores_for_current_week), registered only
while a game is inside its kickoff window. It costs 1 API call against the
full sync's 5, because it reads getNFLScoresOnly directly instead of
building NflApi.games. A flag on the existing job could not have saved
anything -- by the time it applies, the calls are already made.
getNFLGamesForWeek carries a stale gameStatusCode; status now comes from
getNFLScoresOnly, with fixtures where the two endpoints disagree.
New jobstate table plus an APScheduler listener records outcome, last run
and last success per job. The admin schedule page shows a light per job,
red when nothing has succeeded within two of its own periods.
Uptime Kuma removed; it only ever reported "I ran".
Cold start no longer 500s every route: an empty week table renders a 503
setting-up page, and admin pages stay reachable so the sync can be kicked.
Admin lives at /v2/admin now, with a schedule view and per-job Run now.

Deferred: award scheduling (#420).

Resolves #410

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

Every job used to be registered at startup and left alone, so cadence was fixed at boot and nothing re-derived it. Now lifespan registers exactly one job -- the dispatcher -- which runs every minute, reads state and converges the schedule toward what that state calls for. Registration is compare-then-replace rather than unconditional re-add: add_job(replace_existing=True) recomputes next_run_time as now + interval, so a per-minute dispatcher would starve an hourly job forever. Notable pieces: - Score polling is its own job (sync_scores_for_current_week), registered only while a game is inside its kickoff window. It costs 1 API call against the full sync's 5, because it reads getNFLScoresOnly directly instead of building NflApi.games. A flag on the existing job could not have saved anything -- by the time it applies, the calls are already made. - getNFLGamesForWeek carries a stale gameStatusCode; status now comes from getNFLScoresOnly, with fixtures where the two endpoints disagree. - New jobstate table plus an APScheduler listener records outcome, last run and last success per job. The admin schedule page shows a light per job, red when nothing has succeeded within two of its own periods. - Uptime Kuma removed; it only ever reported "I ran". - Cold start no longer 500s every route: an empty week table renders a 503 setting-up page, and admin pages stay reachable so the sync can be kicked. - Admin lives at /v2/admin now, with a schedule view and per-job Run now. Deferred: award scheduling (#420). Resolves #410 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

johnsturgeon added 1 commit

2026-08-24 17:28:18 +02:00

All scheduling now runs through a dispatcher

Claude Code Review / claude-review (pull_request) Successful in 4m32s

Details

Python FastAPI Jinja Linting / build (pull_request) Successful in 32s

Details

Tests / pytest (pull_request) Successful in 19s

Details

5338ec2df1

Every job used to be registered at startup and left alone, so cadence was
fixed at boot and nothing re-derived it. Now lifespan registers exactly one
job -- the dispatcher -- which runs every minute, reads state and converges
the schedule toward what that state calls for.

Registration is compare-then-replace rather than unconditional re-add:
add_job(replace_existing=True) recomputes next_run_time as now + interval, so
a per-minute dispatcher would starve an hourly job forever.

Notable pieces:

- Score polling is its own job (sync_scores_for_current_week), registered only
  while a game is inside its kickoff window. It costs 1 API call against the
  full sync's 5, because it reads getNFLScoresOnly directly instead of
  building NflApi.games. A flag on the existing job could not have saved
  anything -- by the time it applies, the calls are already made.
- getNFLGamesForWeek carries a stale gameStatusCode; status now comes from
  getNFLScoresOnly, with fixtures where the two endpoints disagree.
- New jobstate table plus an APScheduler listener records outcome, last run
  and last success per job. The admin schedule page shows a light per job,
  red when nothing has succeeded within two of its own periods.
- Uptime Kuma removed; it only ever reported "I ran".
- Cold start no longer 500s every route: an empty week table renders a 503
  setting-up page, and admin pages stay reachable so the sync can be kicked.
- Admin lives at /v2/admin now, with a schedule view and per-job Run now.

Deferred: award scheduling (#420).

Resolves #410

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

forgejo-actions commented

2026-08-24 17:32:49 +02:00

This is a large, well-structured PR. I've reviewed the dispatcher, the new/changed jobs, the API changes, the routing/cold-start handling, and checked for dangling references to the deleted modules. No blocking defects; a few things worth confirming.

Solid refactor — the compare-then-replace convergence logic is sound, the cold-start 503 path is well thought through, and the status-from-scores fix is correctly threaded through both NflApiGame and the score sync with fixtures that encode the real disagreement. Below are a handful of things worth a second look, mostly around behavior changes and test coverage rather than outright bugs.

Worth fixing

Awards are no longer recalculated on any schedule, and picks submission no longer triggers a recalc. app/main.py:357 drops the add_job that used to enqueue update_all_awards after a player submits picks, and schedule_award_updates is gone from the scheduler with no replacement in app/jobs/dispatcher.py:211. The description notes award scheduling is deferred to #420, but the net effect is that PlayerAward records will go stale for the whole season until #420 lands — worth confirming that's the intended interim state and not an oversight, since nothing in the UI will hint that awards have stopped updating.
update_player_records only runs on a final transition caught inside the live window. app/jobs/sync_scores_for_current_week.py recomputes player records only when newly_final flips, and the score-sync job only exists while a game is within [kickoff-5min, kickoff+5h] (_in_live_window, app/jobs/dispatcher.py:24). The old update_a_game job polled for 8h and the hourly full sync (sync_games_for_current_week) does not call update_player_records at all. A game that finalizes more than 5h after kickoff (or a final the window otherwise misses) leaves player records stale until the next final anywhere triggers a full-season rebuild. Practically rare, but there's now no fallback reconciliation path.
Test coverage gap on the core of the PR. The added tests (tests/test_tank01_api.py::TestStatusComesFromTheScoresEndpoint) only cover the status-source change. There are no tests for the pieces this PR is actually about: the convergence/compare-then-replace logic and _in_live_window in app/jobs/dispatcher.py, sync_scores_for_current_week's newly_final gating, lock_odds (including the OddsLockException path), or the _record_job_run listener / JobState health rendering. _in_live_window, _job_health, and the score add/remove branching are all pure-ish and cheap to unit-test.

Nits

Inconsistent NflApi construction costs an extra API call. sync_games_for_current_week builds NflApi(week_no=..., season_type=...) (app/jobs/sync_games_for_current_week.py:84) without season, so current_info can't short-circuit and spends a /getNFLCurrentInfo round trip — whereas sync_scores_for_current_week passes all three overrides and skips it. Passing season=current_week.season here would make it consistent and one call cheaper.
JobState rows for the nag jobs accumulate unbounded. Nag job ids are per-week (s{season}:st:{type}:w{week}:d{delta}, app/jobs/dispatcher.py:197), so _record_job_run writes a fresh jobstate row every week that's never cleaned up. Harmless in volume, but the schedule page will slowly fill with dead one-shot rows.
_schedule_score_sync_games_for_current_week omits next_run_time where every sibling _schedule_* passes next_run_time=now (app/jobs/dispatcher.py:107). It's harmless (first run lands at now+1min, and the guard prevents re-adds), but the asymmetry reads like an oversight — a one-line comment or the explicit next_run_time would settle it.
sync_week_state_from_game_status logs the missing-week case at error (app/jobs/sync_week_state_from_game_status.py:42) while every other job treats the same cold-start condition as info/warning. On a fresh DB this will page as an error event through the Sentry logging integration until the first sync lands.

This is a large, well-structured PR. I've reviewed the dispatcher, the new/changed jobs, the API changes, the routing/cold-start handling, and checked for dangling references to the deleted modules. No blocking defects; a few things worth confirming. --- Solid refactor — the compare-then-replace convergence logic is sound, the cold-start 503 path is well thought through, and the status-from-scores fix is correctly threaded through both `NflApiGame` and the score sync with fixtures that encode the real disagreement. Below are a handful of things worth a second look, mostly around behavior changes and test coverage rather than outright bugs. ## Worth fixing - **Awards are no longer recalculated on any schedule, and picks submission no longer triggers a recalc.** `app/main.py:357` drops the `add_job` that used to enqueue `update_all_awards` after a player submits picks, and `schedule_award_updates` is gone from the scheduler with no replacement in `app/jobs/dispatcher.py:211`. The description notes award scheduling is deferred to #420, but the net effect is that `PlayerAward` records will go stale for the whole season until #420 lands — worth confirming that's the intended interim state and not an oversight, since nothing in the UI will hint that awards have stopped updating. - **`update_player_records` only runs on a final transition caught inside the live window.** `app/jobs/sync_scores_for_current_week.py` recomputes player records only when `newly_final` flips, and the score-sync job only exists while a game is within `[kickoff-5min, kickoff+5h]` (`_in_live_window`, `app/jobs/dispatcher.py:24`). The old `update_a_game` job polled for 8h and the hourly full sync (`sync_games_for_current_week`) does *not* call `update_player_records` at all. A game that finalizes more than 5h after kickoff (or a final the window otherwise misses) leaves player records stale until the *next* final anywhere triggers a full-season rebuild. Practically rare, but there's now no fallback reconciliation path. - **Test coverage gap on the core of the PR.** The added tests (`tests/test_tank01_api.py::TestStatusComesFromTheScoresEndpoint`) only cover the status-source change. There are no tests for the pieces this PR is actually about: the convergence/compare-then-replace logic and `_in_live_window` in `app/jobs/dispatcher.py`, `sync_scores_for_current_week`'s `newly_final` gating, `lock_odds` (including the `OddsLockException` path), or the `_record_job_run` listener / `JobState` health rendering. `_in_live_window`, `_job_health`, and the score add/remove branching are all pure-ish and cheap to unit-test. ## Nits - **Inconsistent `NflApi` construction costs an extra API call.** `sync_games_for_current_week` builds `NflApi(week_no=..., season_type=...)` (`app/jobs/sync_games_for_current_week.py:84`) without `season`, so `current_info` can't short-circuit and spends a `/getNFLCurrentInfo` round trip — whereas `sync_scores_for_current_week` passes all three overrides and skips it. Passing `season=current_week.season` here would make it consistent and one call cheaper. - **`JobState` rows for the nag jobs accumulate unbounded.** Nag job ids are per-week (`s{season}:st:{type}:w{week}:d{delta}`, `app/jobs/dispatcher.py:197`), so `_record_job_run` writes a fresh `jobstate` row every week that's never cleaned up. Harmless in volume, but the schedule page will slowly fill with dead one-shot rows. - **`_schedule_score_sync_games_for_current_week` omits `next_run_time`** where every sibling `_schedule_*` passes `next_run_time=now` (`app/jobs/dispatcher.py:107`). It's harmless (first run lands at now+1min, and the guard prevents re-adds), but the asymmetry reads like an oversight — a one-line comment or the explicit `next_run_time` would settle it. - **`sync_week_state_from_game_status` logs the missing-week case at `error`** (`app/jobs/sync_week_state_from_game_status.py:42`) while every other job treats the same cold-start condition as `info`/`warning`. On a fresh DB this will page as an error event through the Sentry logging integration until the first sync lands.

johnsturgeon merged commit e061c1160e into main

2026-08-24 19:44:33 +02:00

johnsturgeon deleted branch phase-4-rewire-the-scheduler-onto-the-week-table-410

2026-08-24 19:44:33 +02:00

johnsturgeon referenced this pull request from a commit

2026-08-24 19:44:34 +02:00

Merge pull request 'All scheduling now runs through a dispatcher' (#421) from phase-4-rewire-the-scheduler-onto-the-week-table-410 into main