fix(beat): cleanup dead-code from prior fallback distinguishing — align with reality
Review-bot PR #520 SHA dc1cd94 нашёл medium discrepancy: `get_all` НЕ raises
на DB unreachable — оно catches Exception и возвращает _DEFAULTS (4 synthetic
rows, scrape_kn + objective_sync enabled). Значит `except` блок в
build_beat_schedule и `rows_seen == 0` branch — dead code, никогда не fires.
Primary fix всё равно работает (operator-disable создаёт реальные rows c
enabled=false → loop их skip'ает → schedule={} → respect). Но контракт в
docstring ("strict, raises on failure") был fiction.
Cleanup (Option A из review):
- Убрана try/except в build_beat_schedule (dead — get_all не raises)
- Убрана rows_seen == 0 branch (dead — get_all всегда returns >=4 rows из _DEFAULTS)
- Сигнатура `_build_beat_schedule_from_db` обратно `-> dict` (не tuple)
- Docstring переписан под реальное поведение: schedule пуст ТОЛЬКО при
operator-initiated disable; DB-unreachable case замаскирован под _DEFAULTS
(синонимично env-fallback по итогу: scrape_kn + objective_sync с дефолтным cron)
Net behavior identical для всех 4 сценариев (operator-disable, normal-config,
DB-unreachable, fresh-install) — но код теперь честный.
This commit is contained in:
parent
dc1cd943e6
commit
cca7dcd029
1 changed files with 24 additions and 33 deletions
|
|
@ -50,21 +50,22 @@ def _default_regions() -> list[int]:
|
||||||
return [int(x.strip()) for x in settings.scrape_kn_default_regions.split(",") if x.strip()]
|
return [int(x.strip()) for x in settings.scrape_kn_default_regions.split(",") if x.strip()]
|
||||||
|
|
||||||
|
|
||||||
def _build_beat_schedule_from_db() -> tuple[dict, int]:
|
def _build_beat_schedule_from_db() -> dict:
|
||||||
"""Строит beat_schedule из job_settings БД.
|
"""Строит beat_schedule из job_settings БД.
|
||||||
|
|
||||||
Returns: (schedule, rows_seen).
|
Returns: schedule (может быть пустой если все enabled rows без cron или disabled).
|
||||||
- rows_seen > 0 → DB reachable, есть конфигурация job_settings (даже если все disabled)
|
|
||||||
- rows_seen == 0 → DB reachable, но таблица пустая (fresh install до seed'а)
|
|
||||||
- **RAISES** при недоступности БД (table missing, connection refused и т.п.) —
|
|
||||||
caller должен поймать и использовать env-based fallback как safety net.
|
|
||||||
|
|
||||||
Используем `get_all` (strict, raises on failure), НЕ `get_all_safe` — нам важно
|
`get_all` маскирует DB-unreachable case через возврат _DEFAULTS (4 synthetic
|
||||||
отличить "DB unreachable" от "DB сказал ничего не schedule'ить".
|
rows, все enabled=True с дефолтными cron'ами) — это значит "DB unreachable" и
|
||||||
Без этого пара UPDATE job_settings SET enabled=false бессильна:
|
"fresh install (table empty)" неотличимы от "normal с _DEFAULTS". В обоих
|
||||||
`_build_beat_schedule_from_db` вернул бы {} как при DB error → caller fallback'ает
|
случаях schedule НЕ будет пустым — _DEFAULTS дадут scrape_kn + objective_sync.
|
||||||
на env-cron → отключённые jobs снова в schedule. См. инцидент 2026-05-24
|
|
||||||
(WAF cooldown disable scrape_kn → fallback вернул его обратно).
|
КРИТИЧНО для operator-disable scenario: когда DB reachable и оператор сделал
|
||||||
|
`UPDATE job_settings SET enabled=false WHERE job_type IN (...)` — get_all
|
||||||
|
вернёт реальные rows (НЕ _DEFAULTS), loop их пропустит → schedule={}. Caller
|
||||||
|
(build_beat_schedule) НЕ должен срывался на env fallback в этом случае,
|
||||||
|
иначе disable бесполезен. См. инцидент 2026-05-24 (WAF cooldown disable
|
||||||
|
scrape_kn → old code fallback'ал scrape_kn обратно из env).
|
||||||
"""
|
"""
|
||||||
from app.core.db import SessionLocal
|
from app.core.db import SessionLocal
|
||||||
from app.services.job_settings import get_all
|
from app.services.job_settings import get_all
|
||||||
|
|
@ -128,7 +129,7 @@ def _build_beat_schedule_from_db() -> tuple[dict, int]:
|
||||||
}
|
}
|
||||||
logger.info("build_beat_schedule: %s cron=%r queue=%s", jt, cron_str, queue)
|
logger.info("build_beat_schedule: %s cron=%r queue=%s", jt, cron_str, queue)
|
||||||
|
|
||||||
return schedule, len(rows)
|
return schedule
|
||||||
|
|
||||||
|
|
||||||
def _build_beat_schedule_fallback() -> dict:
|
def _build_beat_schedule_fallback() -> dict:
|
||||||
|
|
@ -179,28 +180,18 @@ def _build_beat_schedule_fallback() -> dict:
|
||||||
|
|
||||||
|
|
||||||
def build_beat_schedule() -> dict:
|
def build_beat_schedule() -> dict:
|
||||||
"""Строит beat_schedule:
|
"""Строит beat_schedule из DB (job_settings) + добавляет hardcoded entries.
|
||||||
- Из DB (job_settings) если DB reachable И rows > 0
|
|
||||||
- Env-based fallback ТОЛЬКО когда DB unreachable ИЛИ table пустая (fresh install)
|
|
||||||
- Пустой schedule с rows>0 (все jobs disabled намеренно) — RESPECT, без fallback
|
|
||||||
|
|
||||||
Всегда добавляет hardcoded entries поверх (refresh-analytics, OSM POI/noise,
|
`_build_beat_schedule_from_db` использует `get_all` который сам fallback'ает на
|
||||||
nspd cleanup) — они не управляются через job_settings.
|
_DEFAULTS при DB unreachable / fresh install → возвращает 4 synthetic rows
|
||||||
|
(scrape_kn + objective_sync enabled). То есть schedule из DB-builder ПУСТ
|
||||||
|
только когда оператор явно disabled все cron-able jobs через UPDATE
|
||||||
|
job_settings — это **намерение**, респектим без env-fallback.
|
||||||
|
|
||||||
|
Hardcoded entries (refresh-analytics, OSM POI/noise, nspd cleanup) добавляются
|
||||||
|
ВСЕГДА — они не управляются через job_settings.
|
||||||
"""
|
"""
|
||||||
schedule: dict = {}
|
schedule = _build_beat_schedule_from_db()
|
||||||
rows_seen = 0
|
|
||||||
db_reachable = True
|
|
||||||
try:
|
|
||||||
schedule, rows_seen = _build_beat_schedule_from_db()
|
|
||||||
except Exception as e:
|
|
||||||
logger.error(
|
|
||||||
"build_beat_schedule: DB unreachable: %s — fallback на env vars (safety net)", e
|
|
||||||
)
|
|
||||||
db_reachable = False
|
|
||||||
|
|
||||||
if not db_reachable or rows_seen == 0:
|
|
||||||
# DB недоступна ИЛИ fresh install (job_settings пустая) → env-based safety net
|
|
||||||
return _build_beat_schedule_fallback()
|
|
||||||
|
|
||||||
# Всегда добавляем refresh_analytics (этот job нет в job_settings,
|
# Всегда добавляем refresh_analytics (этот job нет в job_settings,
|
||||||
# он технический и не требует конфигурации через UI).
|
# он технический и не требует конфигурации через UI).
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue