Fix public snapshot runner initialize storage argument
This commit is contained in:
@@ -0,0 +1,70 @@
|
|||||||
|
---
|
||||||
|
id: TASK-245
|
||||||
|
title: Fix public snapshot runner initialize storage argument
|
||||||
|
status: done
|
||||||
|
type: backend
|
||||||
|
team: Backend Senior
|
||||||
|
supporting_teams:
|
||||||
|
- Arquitecto Python
|
||||||
|
roadmap_item: foundation
|
||||||
|
priority: high
|
||||||
|
---
|
||||||
|
|
||||||
|
# TASK-245 - Fix public snapshot runner initialize storage argument
|
||||||
|
|
||||||
|
## Goal
|
||||||
|
|
||||||
|
Fix the deployed manual public jobs that failed with `initialize_rcon_materialized_storage() got an unexpected keyword argument 'ensure_storage'`.
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
TASK-243 added `ensure_storage` gating in the ranking and historical snapshot paths, but one wrapper incorrectly propagated that keyword into `initialize_rcon_materialized_storage()`, whose signature only accepts `db_path`.
|
||||||
|
|
||||||
|
The failure was visible when running:
|
||||||
|
|
||||||
|
- `python -m app.historical_runner --public-job ranking-weekly`
|
||||||
|
- `python -m app.historical_runner --public-job ranking-monthly`
|
||||||
|
- `python -m app.historical_runner --public-job historical-weekly`
|
||||||
|
- `python -m app.historical_runner --public-job historical-monthly`
|
||||||
|
|
||||||
|
## Steps
|
||||||
|
|
||||||
|
1. Audited every `ensure_storage=` path around public snapshot generation.
|
||||||
|
2. Confirmed that `initialize_rcon_materialized_storage()` does not accept that keyword.
|
||||||
|
3. Moved the gating logic to a local path resolver in `rcon_historical_leaderboards`.
|
||||||
|
4. Kept `connect_postgres_compat(initialize=False)` for follow-up read/write connections after the one-time initialization point.
|
||||||
|
5. Added regression tests for both enabled and disabled initialization flows.
|
||||||
|
|
||||||
|
## Files to Read First
|
||||||
|
|
||||||
|
- `AGENTS.md`
|
||||||
|
- `ai/architecture-index.md`
|
||||||
|
- `ai/repo-context.md`
|
||||||
|
- `backend/app/historical_runner.py`
|
||||||
|
- `backend/app/rcon_historical_leaderboards.py`
|
||||||
|
- `backend/app/rcon_admin_log_materialization.py`
|
||||||
|
|
||||||
|
## Expected Files to Modify
|
||||||
|
|
||||||
|
- `backend/app/rcon_historical_leaderboards.py`
|
||||||
|
- `backend/tests/test_historical_snapshot_refresh.py`
|
||||||
|
- `docs/public-snapshot-refresh-schedule.md`
|
||||||
|
|
||||||
|
## Constraints
|
||||||
|
|
||||||
|
- Keep public GET endpoints snapshot-only.
|
||||||
|
- Do not touch frontend, assets or server configuration.
|
||||||
|
- Preserve the TASK-243 fixes for JSON serialization, duplicate ranking items and monthly MVP V2 fallback.
|
||||||
|
|
||||||
|
## Validation
|
||||||
|
|
||||||
|
- `python -m compileall backend/app`
|
||||||
|
- `cd backend; python -m unittest tests.test_historical_snapshot_refresh`
|
||||||
|
- `cd backend; python -m unittest tests.test_current_match_payload tests.test_rcon_admin_log_storage tests.test_historical_snapshot_refresh`
|
||||||
|
|
||||||
|
## Outcome
|
||||||
|
|
||||||
|
- Manual public jobs no longer pass an invalid keyword to `initialize_rcon_materialized_storage()`.
|
||||||
|
- `ensure_storage=True` still initializes materialized storage once through the wrapper layer.
|
||||||
|
- `ensure_storage=False` now skips the base initializer cleanly and relies on the existing non-DDL connection path.
|
||||||
|
- Regression coverage now asserts both enabled and disabled initialization flows.
|
||||||
@@ -107,7 +107,7 @@ def initialize_ranking_snapshot_storage(
|
|||||||
ensure_storage: bool = True,
|
ensure_storage: bool = True,
|
||||||
) -> Path:
|
) -> Path:
|
||||||
"""Create ranking snapshot tables used by weekly/monthly public ranking reads."""
|
"""Create ranking snapshot tables used by weekly/monthly public ranking reads."""
|
||||||
resolved_path = initialize_rcon_materialized_storage(
|
resolved_path = _resolve_materialized_storage_path(
|
||||||
db_path=db_path,
|
db_path=db_path,
|
||||||
ensure_storage=ensure_storage,
|
ensure_storage=ensure_storage,
|
||||||
)
|
)
|
||||||
@@ -430,6 +430,16 @@ def _resolve_ranking_snapshot_sqlite_path(*, db_path: Path | None = None) -> Pat
|
|||||||
return db_path if db_path is not None else get_storage_path()
|
return db_path if db_path is not None else get_storage_path()
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_materialized_storage_path(
|
||||||
|
*,
|
||||||
|
db_path: Path | None = None,
|
||||||
|
ensure_storage: bool = True,
|
||||||
|
) -> Path:
|
||||||
|
if ensure_storage:
|
||||||
|
return initialize_rcon_materialized_storage(db_path=db_path)
|
||||||
|
return _resolve_ranking_snapshot_sqlite_path(db_path=db_path)
|
||||||
|
|
||||||
|
|
||||||
def _build_missing_ranking_snapshot_result(
|
def _build_missing_ranking_snapshot_result(
|
||||||
*,
|
*,
|
||||||
timeframe: str,
|
timeframe: str,
|
||||||
@@ -566,7 +576,7 @@ def list_rcon_materialized_leaderboard(
|
|||||||
normalized_limit = max(1, int(limit or 10))
|
normalized_limit = max(1, int(limit or 10))
|
||||||
anchor = _as_utc(now or datetime.now(timezone.utc))
|
anchor = _as_utc(now or datetime.now(timezone.utc))
|
||||||
|
|
||||||
resolved_path = initialize_rcon_materialized_storage(
|
resolved_path = _resolve_materialized_storage_path(
|
||||||
db_path=db_path,
|
db_path=db_path,
|
||||||
ensure_storage=ensure_storage,
|
ensure_storage=ensure_storage,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -50,6 +50,8 @@ from app.rcon_historical_read_model import (
|
|||||||
_calculate_duration_seconds,
|
_calculate_duration_seconds,
|
||||||
)
|
)
|
||||||
from app.rcon_historical_leaderboards import _dedupe_snapshot_rows
|
from app.rcon_historical_leaderboards import _dedupe_snapshot_rows
|
||||||
|
from app.rcon_historical_leaderboards import initialize_ranking_snapshot_storage
|
||||||
|
from app.rcon_historical_leaderboards import list_rcon_materialized_leaderboard
|
||||||
|
|
||||||
|
|
||||||
class HistoricalSnapshotRefreshTests(unittest.TestCase):
|
class HistoricalSnapshotRefreshTests(unittest.TestCase):
|
||||||
@@ -306,6 +308,86 @@ class HistoricalSnapshotRefreshTests(unittest.TestCase):
|
|||||||
self.assertEqual(payload["event_coverage"]["reason"], "player-event-raw-ledger-missing")
|
self.assertEqual(payload["event_coverage"]["reason"], "player-event-raw-ledger-missing")
|
||||||
self.assertEqual(payload["items"], [])
|
self.assertEqual(payload["items"], [])
|
||||||
|
|
||||||
|
def test_initialize_ranking_snapshot_storage_uses_materialized_initializer_without_invalid_keyword(self) -> None:
|
||||||
|
db_path = Path("dummy.sqlite3")
|
||||||
|
with (
|
||||||
|
patch(
|
||||||
|
"app.rcon_historical_leaderboards.initialize_rcon_materialized_storage",
|
||||||
|
return_value=db_path,
|
||||||
|
) as materialized_initializer,
|
||||||
|
patch("app.rcon_historical_leaderboards.use_postgres_rcon_storage", return_value=False),
|
||||||
|
patch(
|
||||||
|
"app.rcon_historical_leaderboards.connect_sqlite_writer",
|
||||||
|
return_value=sqlite3.connect(":memory:"),
|
||||||
|
),
|
||||||
|
):
|
||||||
|
resolved_path = initialize_ranking_snapshot_storage(db_path=db_path, ensure_storage=True)
|
||||||
|
|
||||||
|
materialized_initializer.assert_called_once_with(db_path=db_path)
|
||||||
|
self.assertEqual(resolved_path, db_path)
|
||||||
|
|
||||||
|
def test_initialize_ranking_snapshot_storage_skips_materialized_initializer_when_disabled(self) -> None:
|
||||||
|
db_path = Path("dummy.sqlite3")
|
||||||
|
with (
|
||||||
|
patch("app.rcon_historical_leaderboards.initialize_rcon_materialized_storage") as materialized_initializer,
|
||||||
|
patch("app.rcon_historical_leaderboards.use_postgres_rcon_storage", return_value=False),
|
||||||
|
patch(
|
||||||
|
"app.rcon_historical_leaderboards.connect_sqlite_writer",
|
||||||
|
return_value=sqlite3.connect(":memory:"),
|
||||||
|
),
|
||||||
|
):
|
||||||
|
resolved_path = initialize_ranking_snapshot_storage(db_path=db_path, ensure_storage=False)
|
||||||
|
|
||||||
|
materialized_initializer.assert_not_called()
|
||||||
|
self.assertEqual(resolved_path, db_path)
|
||||||
|
|
||||||
|
def test_list_rcon_materialized_leaderboard_skips_materialized_initializer_when_disabled(self) -> None:
|
||||||
|
db_path = Path("dummy.sqlite3")
|
||||||
|
window = {
|
||||||
|
"start": datetime(2026, 6, 9, tzinfo=timezone.utc),
|
||||||
|
"end": datetime(2026, 6, 10, tzinfo=timezone.utc),
|
||||||
|
"days": 1,
|
||||||
|
"kind": "current-week",
|
||||||
|
"label": "Semana actual",
|
||||||
|
"selection_reason": "test",
|
||||||
|
"current_week_start": datetime(2026, 6, 9, tzinfo=timezone.utc),
|
||||||
|
"current_week_closed_matches": 1,
|
||||||
|
"previous_week_closed_matches": 2,
|
||||||
|
"current_month_start": datetime(2026, 6, 1, tzinfo=timezone.utc),
|
||||||
|
"selected_month_start": datetime(2026, 6, 1, tzinfo=timezone.utc),
|
||||||
|
"selected_month_end": datetime(2026, 6, 10, tzinfo=timezone.utc),
|
||||||
|
"current_month_closed_matches": 3,
|
||||||
|
"previous_month_closed_matches": 4,
|
||||||
|
"sufficient_sample": {"minimum_closed_matches": 1},
|
||||||
|
}
|
||||||
|
fake_connection = object()
|
||||||
|
with (
|
||||||
|
patch("app.rcon_historical_leaderboards.initialize_rcon_materialized_storage") as materialized_initializer,
|
||||||
|
patch(
|
||||||
|
"app.rcon_historical_leaderboards._connect_scope",
|
||||||
|
return_value=nullcontext(fake_connection),
|
||||||
|
) as connect_scope,
|
||||||
|
patch("app.rcon_historical_leaderboards.select_leaderboard_window", return_value=window),
|
||||||
|
patch("app.rcon_historical_leaderboards._fetch_leaderboard_rows", return_value=[]),
|
||||||
|
patch(
|
||||||
|
"app.rcon_historical_leaderboards._fetch_source_range",
|
||||||
|
return_value=(None, None),
|
||||||
|
),
|
||||||
|
):
|
||||||
|
payload = list_rcon_materialized_leaderboard(
|
||||||
|
server_key="all-servers",
|
||||||
|
timeframe="weekly",
|
||||||
|
metric="kills",
|
||||||
|
limit=10,
|
||||||
|
ensure_storage=False,
|
||||||
|
db_path=db_path,
|
||||||
|
now=datetime(2026, 6, 10, tzinfo=timezone.utc),
|
||||||
|
)
|
||||||
|
|
||||||
|
materialized_initializer.assert_not_called()
|
||||||
|
connect_scope.assert_called_once_with(db_path, db_path=db_path, initialize=False)
|
||||||
|
self.assertEqual(payload["items"], [])
|
||||||
|
|
||||||
def test_historical_leaderboard_snapshot_does_not_runtime_enrich_public_request(self) -> None:
|
def test_historical_leaderboard_snapshot_does_not_runtime_enrich_public_request(self) -> None:
|
||||||
snapshot = {
|
snapshot = {
|
||||||
"generated_at": "2026-06-10T04:00:00Z",
|
"generated_at": "2026-06-10T04:00:00Z",
|
||||||
|
|||||||
@@ -102,6 +102,7 @@ Manual public jobs executed with `python -m app.historical_runner --public-job .
|
|||||||
- ranking snapshot rows are deduplicated by `player_id` before inserting `ranking_snapshot_items`
|
- ranking snapshot rows are deduplicated by `player_id` before inserting `ranking_snapshot_items`
|
||||||
- PostgreSQL snapshot storage initialization runs once at the start of the heavy job, then substeps reuse non-DDL connections
|
- PostgreSQL snapshot storage initialization runs once at the start of the heavy job, then substeps reuse non-DDL connections
|
||||||
- missing `player_event_raw_ledger` no longer aborts `historical-monthly`; the monthly MVP V2 slice degrades to an empty payload with `event_coverage.ready = false`
|
- missing `player_event_raw_ledger` no longer aborts `historical-monthly`; the monthly MVP V2 slice degrades to an empty payload with `event_coverage.ready = false`
|
||||||
|
- materialized storage initialization gating stays in the leaderboard wrapper layer; `initialize_rcon_materialized_storage()` is called without extra keywords only when initialization is actually required
|
||||||
|
|
||||||
This keeps manual validation commands useful without hiding partial failures inside the job payload.
|
This keeps manual validation commands useful without hiding partial failures inside the job payload.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user