Fix ranking snapshot generator PostgreSQL default
This commit is contained in:
@@ -0,0 +1,134 @@
|
||||
---
|
||||
id: TASK-195-fix-ranking-snapshot-generator-postgres-default
|
||||
title: Fix ranking snapshot generator PostgreSQL default
|
||||
status: done
|
||||
type: backend
|
||||
team: Backend Senior
|
||||
supporting_teams:
|
||||
- Arquitecto de Base de Datos
|
||||
- Arquitecto Python
|
||||
roadmap_item: foundation
|
||||
priority: high
|
||||
---
|
||||
|
||||
# TASK-195 - Fix ranking snapshot generator PostgreSQL default
|
||||
|
||||
## Goal
|
||||
|
||||
Correct the weekly/monthly ranking snapshot CLI so production generation uses PostgreSQL by default, matching the operational `/api/ranking` read path.
|
||||
|
||||
## Context
|
||||
|
||||
`TASK-194` added a manual weekly/monthly snapshot generator, but the CLI currently passes `db_path=get_storage_path()` into `generate_ranking_snapshot(...)`. In this repository, `use_postgres_rcon_storage(...)` only enables PostgreSQL when `explicit_sqlite_path is None` and `HLL_BACKEND_DATABASE_URL` exists, so the current CLI forces SQLite while `/api/ranking` reads PostgreSQL.
|
||||
|
||||
This causes operational commands to generate `ready` snapshots with `item_count=0` and `source_matches_count=0` in SQLite even when runtime fallback over PostgreSQL returns ranking players.
|
||||
|
||||
Preserve the current product identity: Spanish-speaking HLL Vietnam community, military/Vietnam/tactical/sober visual direction and controlled repository evolution.
|
||||
|
||||
## Steps
|
||||
|
||||
1. Read the listed files first.
|
||||
2. Change the ranking snapshot CLI to use PostgreSQL by default in operational mode.
|
||||
3. Keep SQLite available only as an explicit local-development override if needed.
|
||||
4. Validate the CLI default path, repository scripts and documentation.
|
||||
5. Document root cause, fix and production validation steps.
|
||||
|
||||
## Files to Read First
|
||||
|
||||
- `AGENTS.md`
|
||||
- `ai/repo-context.md`
|
||||
- `ai/architecture-index.md`
|
||||
- `backend/app/config.py`
|
||||
- `backend/app/rcon_historical_leaderboards.py`
|
||||
- `backend/app/postgres_rcon_storage.py`
|
||||
- `scripts/run-stats-validation.ps1`
|
||||
- `docs/ranking-snapshot-read-model-plan.md`
|
||||
- `ai/tasks/done/TASK-194-add-weekly-monthly-ranking-snapshot-generator.md`
|
||||
|
||||
## Expected Files to Modify
|
||||
|
||||
- `backend/app/rcon_historical_leaderboards.py`
|
||||
- `scripts/run-stats-validation.ps1`
|
||||
- `docs/ranking-snapshot-read-model-plan.md`
|
||||
- `ai/tasks/done/TASK-195-fix-ranking-snapshot-generator-postgres-default.md`
|
||||
|
||||
## Constraints
|
||||
|
||||
- Keep the change minimal.
|
||||
- Do not modify frontend, assets or design.
|
||||
- Do not change public endpoint behavior beyond the intended generator hotfix.
|
||||
- Keep annual ranking behavior unchanged.
|
||||
- Do not reactivate Elo/MMR.
|
||||
- Do not reintroduce Comunidad Hispana #03.
|
||||
- Keep PostgreSQL as the operational default and SQLite only as an explicit local-development mode.
|
||||
|
||||
## Validation
|
||||
|
||||
Before completing the task ensure:
|
||||
|
||||
- `powershell -ExecutionPolicy Bypass -File scripts/run-stats-validation.ps1`
|
||||
- `powershell -ExecutionPolicy Bypass -File scripts/run-integration-tests.ps1`
|
||||
- local import or parser validation proves the CLI no longer passes `get_storage_path()` by default
|
||||
- local validation proves `generate_ranking_snapshot(..., db_path=None)` uses PostgreSQL when `HLL_BACKEND_DATABASE_URL` is configured
|
||||
- `git diff --name-only` matches the expected scope
|
||||
|
||||
## Outcome
|
||||
|
||||
Root cause:
|
||||
|
||||
- the manual CLI in `backend/app/rcon_historical_leaderboards.py` called `generate_ranking_snapshot(..., db_path=get_storage_path())`
|
||||
- that forced `explicit_sqlite_path != None`
|
||||
- `use_postgres_rcon_storage(...)` therefore disabled PostgreSQL even when `HLL_BACKEND_DATABASE_URL` was configured
|
||||
- operational `/api/ranking` reads PostgreSQL, so the generator and the public ranking endpoint diverged onto different storage backends
|
||||
|
||||
Applied change:
|
||||
|
||||
- `backend/app/rcon_historical_leaderboards.py`
|
||||
- removed the default CLI path that forced SQLite
|
||||
- changed the operational default to `db_path=None`
|
||||
- added explicit `--sqlite-path <path>` override for local development only
|
||||
- `scripts/run-stats-validation.ps1`
|
||||
- now validates that the CLI default passes `db_path=None`
|
||||
- now validates that PostgreSQL selection activates when `HLL_BACKEND_DATABASE_URL` is configured and no explicit SQLite path is provided
|
||||
- `docs/ranking-snapshot-read-model-plan.md`
|
||||
- now documents PostgreSQL as the operational default
|
||||
- now documents `--sqlite-path` as an explicit local override only
|
||||
|
||||
Previous vs new behavior:
|
||||
|
||||
- before:
|
||||
- `python -m app.rcon_historical_leaderboards generate-ranking-snapshot ...`
|
||||
- forced SQLite by default
|
||||
- could generate empty snapshots operationally while `/api/ranking` fallback over PostgreSQL still returned players
|
||||
- now:
|
||||
- the same command uses PostgreSQL by default when `HLL_BACKEND_DATABASE_URL` is configured
|
||||
- SQLite is used only when the operator passes `--sqlite-path`
|
||||
|
||||
Validation executed:
|
||||
|
||||
- `powershell -ExecutionPolicy Bypass -File scripts/run-stats-validation.ps1`
|
||||
- `powershell -ExecutionPolicy Bypass -File scripts/run-integration-tests.ps1`
|
||||
|
||||
Validation notes:
|
||||
|
||||
- live backend HTTP at `http://127.0.0.1:8000` was not available in this environment
|
||||
- route and CLI validation completed through local Python imports and repository validation scripts
|
||||
- no local PostgreSQL instance was required because the validation proved backend selection by inspection and monkeypatched connection-path checks
|
||||
|
||||
Final recommended Docker command:
|
||||
|
||||
- `docker compose exec backend python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20`
|
||||
|
||||
How to validate in production:
|
||||
|
||||
1. run the Docker command above in the production backend container
|
||||
2. query PostgreSQL `ranking_snapshots` for the selected `(timeframe, server_id, metric, window_start, window_end)`
|
||||
3. confirm `item_count > 0` and `source_matches_count > 0` for covered windows
|
||||
4. call `/api/ranking?timeframe=weekly&server_id=all&metric=kills&limit=20`
|
||||
5. confirm `snapshot_status=ready` and `fallback_used=false`
|
||||
|
||||
## Change Budget
|
||||
|
||||
- Prefer fewer than 5 modified files.
|
||||
- Prefer changes under 200 lines when feasible.
|
||||
- Split the work into follow-up tasks if limits are exceeded.
|
||||
@@ -10,8 +10,7 @@ from datetime import datetime, timedelta, timezone
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
|
||||
from .config import get_storage_path, use_postgres_rcon_storage
|
||||
from .config import get_historical_weekly_fallback_min_matches
|
||||
from .config import get_historical_weekly_fallback_min_matches, use_postgres_rcon_storage
|
||||
from .historical_storage import ALL_SERVERS_SLUG
|
||||
from .rcon_admin_log_materialization import (
|
||||
MATCH_RESULT_SOURCE,
|
||||
@@ -1256,6 +1255,12 @@ def _main(argv: list[str] | None = None) -> int:
|
||||
choices=SNAPSHOT_GENERATOR_METRICS,
|
||||
)
|
||||
generate_parser.add_argument("--limit", type=int, default=20)
|
||||
generate_parser.add_argument(
|
||||
"--sqlite-path",
|
||||
type=Path,
|
||||
default=None,
|
||||
help="explicit local SQLite override; default operational mode uses PostgreSQL when configured",
|
||||
)
|
||||
generate_parser.add_argument(
|
||||
"--no-replace-existing",
|
||||
action="store_false",
|
||||
@@ -1272,7 +1277,7 @@ def _main(argv: list[str] | None = None) -> int:
|
||||
metric=args.metric,
|
||||
limit=args.limit,
|
||||
replace_existing=args.replace_existing,
|
||||
db_path=get_storage_path(),
|
||||
db_path=args.sqlite_path,
|
||||
)
|
||||
print(json.dumps({"status": "ok", "data": payload}, ensure_ascii=True, indent=2))
|
||||
return 0
|
||||
|
||||
@@ -253,12 +253,24 @@ Manual generator entrypoint:
|
||||
python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20
|
||||
```
|
||||
|
||||
Operational default:
|
||||
- when `HLL_BACKEND_DATABASE_URL` is configured, the CLI uses PostgreSQL by default
|
||||
- SQLite is no longer the default operational target for snapshot generation
|
||||
- local SQLite generation remains available only through an explicit override such as:
|
||||
|
||||
```bash
|
||||
python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20 --sqlite-path backend/data/hll_vietnam_dev.sqlite3
|
||||
```
|
||||
|
||||
Docker form:
|
||||
|
||||
```bash
|
||||
docker compose exec backend python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20
|
||||
```
|
||||
|
||||
Operational expectation:
|
||||
- the recommended Docker command should generate weekly/monthly snapshots in PostgreSQL, matching the `/api/ranking` production read path
|
||||
|
||||
Supported manual parameters:
|
||||
- `timeframe`: `weekly`, `monthly`
|
||||
- `server-key`: `all`, `all-servers`, `comunidad-hispana-01`, `comunidad-hispana-02`
|
||||
|
||||
@@ -117,12 +117,15 @@ import json
|
||||
import os
|
||||
import sqlite3
|
||||
import sys
|
||||
from contextlib import contextmanager, redirect_stdout
|
||||
from datetime import datetime, timezone
|
||||
from io import StringIO
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, "backend")
|
||||
|
||||
from app.routes import resolve_get_payload
|
||||
from app.config import use_postgres_rcon_storage
|
||||
import app.postgres_rcon_storage as postgres_rcon_storage
|
||||
import app.rcon_historical_leaderboards as ranking_leaderboards
|
||||
|
||||
@@ -284,6 +287,90 @@ def validate_postgres_ranking_snapshot_schema_path():
|
||||
)
|
||||
|
||||
|
||||
def validate_ranking_snapshot_cli_defaults():
|
||||
original_generate_ranking_snapshot = ranking_leaderboards.generate_ranking_snapshot
|
||||
captured = {}
|
||||
|
||||
def fake_generate_ranking_snapshot(**kwargs):
|
||||
captured.update(kwargs)
|
||||
return {"status": "ok", "snapshot": None, "items": []}
|
||||
|
||||
ranking_leaderboards.generate_ranking_snapshot = fake_generate_ranking_snapshot
|
||||
|
||||
try:
|
||||
stdout_buffer = StringIO()
|
||||
with redirect_stdout(stdout_buffer):
|
||||
exit_code = ranking_leaderboards._main([
|
||||
"generate-ranking-snapshot",
|
||||
"--timeframe", "weekly",
|
||||
"--server-key", "all",
|
||||
"--metric", "kills",
|
||||
"--limit", "20",
|
||||
])
|
||||
require(exit_code == 0, "Ranking snapshot CLI should exit 0 for a valid command.")
|
||||
require(
|
||||
captured.get("db_path") is None,
|
||||
"Ranking snapshot CLI should use PostgreSQL-compatible db_path=None by default.",
|
||||
)
|
||||
|
||||
captured.clear()
|
||||
with redirect_stdout(stdout_buffer):
|
||||
exit_code = ranking_leaderboards._main([
|
||||
"generate-ranking-snapshot",
|
||||
"--timeframe", "weekly",
|
||||
"--server-key", "all",
|
||||
"--metric", "kills",
|
||||
"--limit", "20",
|
||||
"--sqlite-path", "backend/data/hll_vietnam_dev.sqlite3",
|
||||
])
|
||||
require(exit_code == 0, "Ranking snapshot CLI with --sqlite-path should exit 0.")
|
||||
require(
|
||||
captured.get("db_path") == Path("backend/data/hll_vietnam_dev.sqlite3"),
|
||||
"Ranking snapshot CLI should pass the explicit --sqlite-path override through to generate_ranking_snapshot.",
|
||||
)
|
||||
finally:
|
||||
ranking_leaderboards.generate_ranking_snapshot = original_generate_ranking_snapshot
|
||||
|
||||
|
||||
def validate_ranking_snapshot_postgres_selection():
|
||||
original_database_url = os.environ.get("HLL_BACKEND_DATABASE_URL")
|
||||
original_connect_postgres_compat = postgres_rcon_storage.connect_postgres_compat
|
||||
calls = {"postgres_connect": 0}
|
||||
|
||||
@contextmanager
|
||||
def fake_connect_postgres_compat():
|
||||
calls["postgres_connect"] += 1
|
||||
yield object()
|
||||
|
||||
os.environ["HLL_BACKEND_DATABASE_URL"] = "postgresql://validation-user:validation-pass@127.0.0.1:5432/hll_validation"
|
||||
postgres_rcon_storage.connect_postgres_compat = fake_connect_postgres_compat
|
||||
|
||||
try:
|
||||
require(
|
||||
use_postgres_rcon_storage(explicit_sqlite_path=None) is True,
|
||||
"PostgreSQL storage should be selected when DATABASE_URL is configured and no explicit SQLite path is provided.",
|
||||
)
|
||||
require(
|
||||
use_postgres_rcon_storage(explicit_sqlite_path=Path("backend/data/hll_vietnam_dev.sqlite3")) is False,
|
||||
"Explicit SQLite paths should still disable PostgreSQL storage selection.",
|
||||
)
|
||||
with ranking_leaderboards._connect_write_scope(
|
||||
Path("backend/data/hll_vietnam_dev.sqlite3"),
|
||||
db_path=None,
|
||||
) as _connection:
|
||||
pass
|
||||
require(
|
||||
calls["postgres_connect"] == 1,
|
||||
"Ranking snapshot write scope should use PostgreSQL when db_path=None and DATABASE_URL is configured.",
|
||||
)
|
||||
finally:
|
||||
if original_database_url is None:
|
||||
os.environ.pop("HLL_BACKEND_DATABASE_URL", None)
|
||||
else:
|
||||
os.environ["HLL_BACKEND_DATABASE_URL"] = original_database_url
|
||||
postgres_rcon_storage.connect_postgres_compat = original_connect_postgres_compat
|
||||
|
||||
|
||||
def cleanup_snapshot_fixture(db_path):
|
||||
connection = sqlite3.connect(db_path)
|
||||
with connection:
|
||||
@@ -325,6 +412,8 @@ require(health_status == 200, "Route resolver /health should return 200.")
|
||||
require(health_payload.get("status") == "ok", "/health payload should be ok.")
|
||||
|
||||
validate_postgres_ranking_snapshot_schema_path()
|
||||
validate_ranking_snapshot_cli_defaults()
|
||||
validate_ranking_snapshot_postgres_selection()
|
||||
|
||||
kd_metric_sql, _, _ = ranking_leaderboards._resolve_metric_sql("kd_ratio")
|
||||
require(
|
||||
@@ -694,8 +783,10 @@ print(json.dumps({
|
||||
"stats-player-profile",
|
||||
"stats-annual-ranking",
|
||||
"global-ranking",
|
||||
"postgres-ranking-derived-metric-sql",
|
||||
"postgres-ranking-derived-metric-sql",
|
||||
"postgres-ranking-schema-path",
|
||||
"ranking-snapshot-cli-postgres-default",
|
||||
"ranking-snapshot-postgres-selection",
|
||||
"ranking-snapshot-generator",
|
||||
"ranking-snapshot-ready",
|
||||
"ranking-snapshot-missing",
|
||||
|
||||
Reference in New Issue
Block a user