From e35e3d6de392dbf7ec3ef0d73fe79149424e40d4 Mon Sep 17 00:00:00 2001 From: Nicolas FRYDER Date: Sat, 22 Aug 2026 14:41:59 +0200 Subject: [PATCH] =?UTF-8?q?refactor:=20module=20de=20sant=C3=A9=20partag?= =?UTF-8?q?=C3=A9,=20supervision=20compl=C3=A8te,=20endpoints=20morts=20tr?= =?UTF-8?q?ait=C3=A9s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit health.py existait en deux copies rigoureusement identiques — crawler et geocoder — dont une seule était couverte par des tests. Rien ne signalait une divergence : une correction appliquée d'un seul côté serait passée inaperçue. Le module part dans libs/bm_health.py, embarqué dans les images via un contexte de build ramené à la racine du dépôt, et rendu importable en local par les conftest.py. Un .dockerignore racine évite que node_modules parte dans les images au passage. Les deux images ont été reconstruites et le module vérifié importable à l'exécution dans chacune. La migration 015 annonçait un battement de coeur pour groq-worker, mais aucun n'était jamais écrit : sa ligne n'existait pas, et le bandeau de santé de l'admin ne pouvait donc rien signaler — y compris quand le service était mort. Même trou pour geocoder-enqueue. Les deux écrivent désormais leur état, avec des sondes propres à leur rôle (clé API, progression de leur file). Deux endpoints étaient définis et testés sans qu'aucun bouton ne les appelle. /admin/api/locations/reset-llm était pourtant le seul moyen de relancer les lieux passés en 'manual' après épuisement des tentatives LLM : la fonctionnalité existait sans que personne puisse l'atteindre. Elle rejoint la zone de danger de l'admin. /admin/api/crawl-checkpoints faisait doublon avec /admin/api/live et disparaît, avec ses tests. Co-Authored-By: Claude Opus 5 --- apps/admin/site/app.js | 9 ++ apps/api/src/adminRoutes.js | 15 -- apps/api/test/adminRoutesCoverage.test.js | 1 - .../test/integration/sql.integration.test.js | 2 +- apps/api/test/sqlSyntax.test.js | 1 - apps/crawler/Dockerfile | 7 +- apps/crawler/src/main.py | 2 +- apps/crawler/tests/conftest.py | 7 + apps/crawler/tests/test_health.py | 4 +- apps/geocoder/Dockerfile | 8 +- apps/geocoder/src/enqueue.py | 22 +++ apps/geocoder/src/groq_worker.py | 28 ++++ apps/geocoder/src/health.py | 133 ------------------ apps/geocoder/src/worker.py | 2 +- apps/geocoder/tests/conftest.py | 7 + docker-compose.dev.yml | 12 +- docker-compose.yml | 9 +- .../src/health.py => libs/bm_health.py | 10 +- 18 files changed, 111 insertions(+), 168 deletions(-) delete mode 100644 apps/geocoder/src/health.py rename apps/crawler/src/health.py => libs/bm_health.py (90%) diff --git a/apps/admin/site/app.js b/apps/admin/site/app.js index f57bf88..e848c55 100644 --- a/apps/admin/site/app.js +++ b/apps/admin/site/app.js @@ -259,6 +259,7 @@ async function renderPilotage(token = renderToken) { + @@ -362,6 +363,14 @@ const DANGER_ACTIONS = { body: { include_done: true }, confirm: "Remettre TOUTES les localisations en file, y compris celles déjà résolues ?", }, + // Cette route existait, était testée, et n'était appelée par aucun bouton : + // c'était pourtant le seul moyen de relancer les lieux passés en 'manual' + // après épuisement des tentatives LLM. La fonctionnalité existait sans que + // personne puisse l'atteindre. + "reset-llm": { + url: "/admin/api/locations/reset-llm", + confirm: "Relancer les lieux que le LLM n'a pas su résoudre ?\n\nIls repartent en file de géocodage — appels Geoapify et Groq facturés.", + }, }; function wireDangerZone() { diff --git a/apps/api/src/adminRoutes.js b/apps/api/src/adminRoutes.js index c1d7555..ecd1c07 100644 --- a/apps/api/src/adminRoutes.js +++ b/apps/api/src/adminRoutes.js @@ -356,21 +356,6 @@ export default async function adminRoutes(fastify, opts) { } }); - // ------------------------------------------------------------------ - // Checkpoints - // (l'historique des crawl_run est exposé de façon unifiée par - // GET /admin/api/activity, avec le détail logs via GET /admin/api/logs?run_id=) - // ------------------------------------------------------------------ - fastify.get("/admin/api/crawl-checkpoints", async (req, reply) => { - try { - const r = await pool.query(`SELECT key, value, updated_at FROM crawl_checkpoint ORDER BY key`); - return { ok: true, items: r.rows }; - } catch (err) { - fastify.log.error(err); - return reply.code(500).send({ ok: false, error: "Erreur checkpoints" }); - } - }); - // ------------------------------------------------------------------ // Logs crawler // ------------------------------------------------------------------ diff --git a/apps/api/test/adminRoutesCoverage.test.js b/apps/api/test/adminRoutesCoverage.test.js index 4564c17..d2edff1 100644 --- a/apps/api/test/adminRoutesCoverage.test.js +++ b/apps/api/test/adminRoutesCoverage.test.js @@ -34,7 +34,6 @@ describe("routes de lecture", () => { const READ_ROUTES = [ "/admin/api/stats", "/admin/api/queue", - "/admin/api/crawl-checkpoints", "/admin/api/logs", "/admin/api/geocoding", "/admin/api/llm", diff --git a/apps/api/test/integration/sql.integration.test.js b/apps/api/test/integration/sql.integration.test.js index 986a8bc..51cd126 100644 --- a/apps/api/test/integration/sql.integration.test.js +++ b/apps/api/test/integration/sql.integration.test.js @@ -139,7 +139,7 @@ describe.runIf(!process.env.SKIP_INTEGRATION)("SQL validé par PostgreSQL", () = const ADMIN_GET = [ "/admin/api/stats", "/admin/api/queue", "/admin/api/bands", "/admin/api/bands?q=mayhem&country=NO&genre=black&location_q=oslo&themes_q=war&enriched=true&has_lat=false&has_location=true&has_conflict=true&sort=name&dir=desc", - "/admin/api/bands/1", "/admin/api/crawl-checkpoints", "/admin/api/logs", + "/admin/api/bands/1", "/admin/api/logs", "/admin/api/logs?level=error&run_id=1&min_id=5", "/admin/api/geocoding", "/admin/api/llm", "/admin/api/llm?model=x&only_null=1&q=oslo", "/admin/api/job-triggers", "/admin/api/live", diff --git a/apps/api/test/sqlSyntax.test.js b/apps/api/test/sqlSyntax.test.js index a20e61c..acc0c99 100644 --- a/apps/api/test/sqlSyntax.test.js +++ b/apps/api/test/sqlSyntax.test.js @@ -106,7 +106,6 @@ describe("SQL des routes admin", () => { { url: "/admin/api/bands" }, { url: "/admin/api/bands?q=mayhem&country=NO&genre=black&location_q=oslo&themes_q=war&enriched=true&has_lat=false&has_location=true&has_conflict=true&sort=name&dir=desc&page=2" }, { url: "/admin/api/bands/1" }, - { url: "/admin/api/crawl-checkpoints" }, { url: "/admin/api/logs" }, { url: "/admin/api/logs?level=error&run_id=1&min_id=5&since=2026-01-01" }, { url: "/admin/api/geocoding" }, diff --git a/apps/crawler/Dockerfile b/apps/crawler/Dockerfile index 283aae6..725c8ec 100644 --- a/apps/crawler/Dockerfile +++ b/apps/crawler/Dockerfile @@ -1,10 +1,13 @@ FROM python:3.12-slim WORKDIR /app -COPY requirements.txt . +COPY apps/crawler/requirements.txt . RUN pip install --no-cache-dir -r requirements.txt -COPY src ./src +# Contexte de build = racine du depot, pour embarquer le module partage. +COPY libs/ ./libs/ +COPY apps/crawler/src ./src +ENV PYTHONPATH=/app/libs # Utilisateur non-root (le crawler parse du HTML tiers avec lxml) RUN useradd -m -u 1000 crawler diff --git a/apps/crawler/src/main.py b/apps/crawler/src/main.py index f6d4a8c..e264db2 100644 --- a/apps/crawler/src/main.py +++ b/apps/crawler/src/main.py @@ -22,6 +22,7 @@ import time from datetime import UTC, datetime import schedule +from bm_health import database_probe, freshness_probe, http_probe, maybe_run from .config import ( ENRICH_LIMIT, @@ -33,7 +34,6 @@ from .config import ( ) from .db import claim_job_trigger, finish_job_trigger, get_checkpoint, recover_stuck_runs from .flaresolverr import FlareSolverr -from .health import database_probe, freshness_probe, http_probe, maybe_run from .jobs import run_enrich, run_full_crawl, run_incremental from .ma_http import MASession diff --git a/apps/crawler/tests/conftest.py b/apps/crawler/tests/conftest.py index cb6fa16..4816750 100644 --- a/apps/crawler/tests/conftest.py +++ b/apps/crawler/tests/conftest.py @@ -1,3 +1,10 @@ +# Le module de sante est partage entre les apps (libs/bm_health.py) : en +# Docker il arrive via PYTHONPATH, ici on l'ajoute au chemin d'import. +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[3] / "libs")) + import os import sys from pathlib import Path diff --git a/apps/crawler/tests/test_health.py b/apps/crawler/tests/test_health.py index 3a17620..0f44aef 100644 --- a/apps/crawler/tests/test_health.py +++ b/apps/crawler/tests/test_health.py @@ -8,9 +8,9 @@ de contrôle du tout — d'où le nombre de cas d'échec vérifiés ici. import contextlib +import bm_health as health import pytest -from src import health -from src.health import ( +from bm_health import ( database_probe, freshness_probe, http_probe, diff --git a/apps/geocoder/Dockerfile b/apps/geocoder/Dockerfile index f9b4f41..8ad572a 100644 --- a/apps/geocoder/Dockerfile +++ b/apps/geocoder/Dockerfile @@ -3,11 +3,13 @@ FROM python:3.12-slim WORKDIR /app # Copier et installer les dépendances en premier (meilleur cache Docker) -COPY requirements.txt . +COPY apps/geocoder/requirements.txt . RUN pip install --no-cache-dir -r requirements.txt -# Copier uniquement le code de l'application -COPY src ./src +# Contexte de build = racine du depot, pour embarquer le module partage. +COPY libs/ ./libs/ +COPY apps/geocoder/src ./src +ENV PYTHONPATH=/app/libs # Utilisateur non-root RUN useradd -m -u 1000 geocoder diff --git a/apps/geocoder/src/enqueue.py b/apps/geocoder/src/enqueue.py index 28d5152..da77883 100644 --- a/apps/geocoder/src/enqueue.py +++ b/apps/geocoder/src/enqueue.py @@ -21,6 +21,7 @@ import signal import time import psycopg2 +from bm_health import database_probe, freshness_probe, maybe_run from parser import ( COUNTRY_CENTROIDS, COUNTRY_NAME_TO_ISO2, @@ -254,6 +255,26 @@ def main(): print(f"[enqueue] daemon démarré, poll {POLL_INTERVAL}s, auto toutes les {AUTO_INTERVAL_MIN}min") + # La migration 015 prévoit un battement de cœur par service de fond ; celui + # de l'enqueue n'existait pas. Un daemon mort restait donc invisible dans le + # bandeau de santé de l'admin. + import contextlib + + @contextlib.contextmanager + def get_conn(): + yield conn + + def health_check(): + maybe_run(get_conn, "geocoder-enqueue", [ + ("database", database_probe(get_conn)), + # L'alimentation tourne au moins toutes les AUTO_INTERVAL_MIN + # minutes : au-delà du double, elle ne passe plus. + ("progression", freshness_probe( + get_conn, + "SELECT max(started_at) FROM crawl_run WHERE run_type = 'geocoder_enqueue'", + max(2, (AUTO_INTERVAL_MIN * 2) // 60), "dernière alimentation")), + ]) + last_auto = time.monotonic() with conn.cursor() as cur: @@ -287,6 +308,7 @@ def main(): row = cur.fetchone() if not row: + health_check() _sleep_interruptible(POLL_INTERVAL) continue diff --git a/apps/geocoder/src/groq_worker.py b/apps/geocoder/src/groq_worker.py index 0f59070..1b261b2 100644 --- a/apps/geocoder/src/groq_worker.py +++ b/apps/geocoder/src/groq_worker.py @@ -26,6 +26,7 @@ from datetime import UTC, datetime import psycopg2 import requests +from bm_health import database_probe, freshness_probe, maybe_run # sys.path[0] = src/ quand lancé comme "python src/groq_worker.py" from parser import COUNTRY_NAMES @@ -187,6 +188,32 @@ def main(): min_start = time.monotonic() day_start = datetime.now(UTC).date() + # La migration 015 annonce un battement de cœur pour 'groq-worker', mais + # aucun n'était jamais écrit : sa ligne n'existait pas, et le bandeau de + # santé de l'admin ne pouvait donc rien signaler — y compris quand le + # service était mort. + import contextlib + + @contextlib.contextmanager + def get_conn(): + yield conn + + def health_check(): + maybe_run(get_conn, "groq-worker", [ + ("database", database_probe(get_conn)), + ("groq_cle", lambda: (bool(GROQ_API_KEY), + "clé absente" if not GROQ_API_KEY else "clé présente")), + # Le pipeline doit avancer : au-delà de 24 h sans lieu sorti de la + # file LLM alors qu'il en reste, quelque chose bloque. + ("progression", freshness_probe( + get_conn, + "SELECT max(updated_at) FROM band_locations " + "WHERE geocode_status IN ('queued','manual') AND geocode_tries_llm > 0", + 24, "dernier lieu traité par le LLM")), + ]) + + health_check() + with conn.cursor() as cur: while not _shutdown: # Reset compteurs si nouvelle minute / nouveau jour @@ -224,6 +251,7 @@ def main(): row = cur.fetchone() if not row: print("[groq] rien à traiter, attente 120s") + health_check() _sleep_interruptible(120) continue diff --git a/apps/geocoder/src/health.py b/apps/geocoder/src/health.py deleted file mode 100644 index 0947e81..0000000 --- a/apps/geocoder/src/health.py +++ /dev/null @@ -1,133 +0,0 @@ -""" -Contrôle de santé périodique des services de fond. - -Ces services ne sont pas exposés par Traefik : aucune sonde HTTP externe ne peut -les atteindre. Sans ce module, un crawler dont FlareSolverr est injoignable ou -un worker à court de quota reste muet, et le seul symptôme est l'absence de -données nouvelles — qu'il faut remarquer soi-même. - -Chaque sonde renvoie (nom, ok, détail). Le résultat agrégé est écrit dans -`service_health` (une ligne par service, écrasée), lu par le dashboard admin. - -Aucune sonde ne peut interrompre le service : toute exception est convertie en -échec de sonde. Un contrôle de santé qui fait tomber ce qu'il surveille serait -pire que pas de contrôle du tout. -""" -import logging -import time - -log = logging.getLogger(__name__) - -# Intervalle minimal entre deux contrôles complets. -DEFAULT_INTERVAL_S = 3600 - -_last_run = 0.0 - - -def probe(name, fn): - """Exécute une sonde et convertit toute exception en échec.""" - try: - ok, detail = fn() - return name, bool(ok), (detail or "") - except Exception as e: # noqa: BLE001 - une sonde ne doit jamais propager - return name, False, f"{type(e).__name__}: {e}" - - -def database_probe(get_conn): - """La base répond-elle ?""" - def _run(): - with get_conn() as conn: - with conn.cursor() as cur: - cur.execute("SELECT 1") - cur.fetchone() - return True, "connexion établie" - return _run - - -def http_probe(session_get, url, expect_below=500, timeout=10): - """Une dépendance HTTP répond-elle sans erreur serveur ?""" - def _run(): - r = session_get(url, timeout=timeout) - code = getattr(r, "status_code", 0) - return code < expect_below, f"HTTP {code}" - return _run - - -def freshness_probe(get_conn, sql, max_age_hours, label): - """Les données progressent-elles ? - - `sql` doit renvoyer un unique timestamp (le plus récent). Une base vide - (NULL) n'est pas un échec : c'est un état légitime au premier démarrage. - """ - def _run(): - with get_conn() as conn: - with conn.cursor() as cur: - cur.execute(sql) - row = cur.fetchone() - latest = row[0] if row else None - if latest is None: - return True, f"{label}: aucune donnée pour l'instant" - with get_conn() as conn: - with conn.cursor() as cur: - cur.execute( - "SELECT EXTRACT(EPOCH FROM (now() - %s)) / 3600.0", (latest,) - ) - age_h = float(cur.fetchone()[0]) - return age_h <= max_age_hours, f"{label}: {age_h:.1f} h (seuil {max_age_hours} h)" - return _run - - -def write_health(get_conn, service, results): - """Enregistre le résultat agrégé. N'échoue jamais bruyamment.""" - import json - - checks = {name: ok for name, ok, _ in results} - failed = [f"{name} — {detail}" for name, ok, detail in results if not ok] - ok = not failed - try: - with get_conn() as conn: - with conn.cursor() as cur: - cur.execute( - """ - INSERT INTO service_health (service, ok, checks, error, checked_at) - VALUES (%s, %s, %s::jsonb, %s, now()) - ON CONFLICT (service) DO UPDATE SET - ok = EXCLUDED.ok, - checks = EXCLUDED.checks, - error = EXCLUDED.error, - checked_at = EXCLUDED.checked_at - """, - (service, ok, json.dumps(checks), failed[0] if failed else None), - ) - except Exception as e: # noqa: BLE001 - log.warning(f"[health] écriture impossible: {e}") - return ok, failed - - -def run_checks(get_conn, service, probes, log_event=None): - """Exécute toutes les sondes, enregistre le résultat, journalise les échecs.""" - results = [probe(name, fn) for name, fn in probes] - ok, failed = write_health(get_conn, service, results) - - if failed: - msg = f"[health:{service}] DÉGRADÉ — " + " | ".join(failed) - log.warning(msg) - if log_event: - try: - log_event("warning", msg) - except Exception: # noqa: BLE001, S110 - journaliser un échec de - # journalisation n'apporte rien et risquerait une récursion. - pass - else: - log.info(f"[health:{service}] toutes les sondes sont au vert") - return ok - - -def maybe_run(get_conn, service, probes, interval_s=DEFAULT_INTERVAL_S, log_event=None, force=False): - """Point d'entrée depuis une boucle de service : ne contrôle qu'une fois par intervalle.""" - global _last_run - now = time.monotonic() - if not force and _last_run and (now - _last_run) < interval_s: - return None - _last_run = now - return run_checks(get_conn, service, probes, log_event=log_event) diff --git a/apps/geocoder/src/worker.py b/apps/geocoder/src/worker.py index b95e9f0..3d80be9 100644 --- a/apps/geocoder/src/worker.py +++ b/apps/geocoder/src/worker.py @@ -23,7 +23,7 @@ import time import psycopg2 import requests -from health import database_probe, freshness_probe, http_probe, maybe_run +from bm_health import database_probe, freshness_probe, http_probe, maybe_run from parser import build_fallback_queries GEOAPIFY_API_KEY = os.environ.get("GEOAPIFY_API_KEY", "").strip() diff --git a/apps/geocoder/tests/conftest.py b/apps/geocoder/tests/conftest.py index 6d0de65..596f6a2 100644 --- a/apps/geocoder/tests/conftest.py +++ b/apps/geocoder/tests/conftest.py @@ -1,3 +1,10 @@ +# Le module de sante est partage entre les apps (libs/bm_health.py) : en +# Docker il arrive via PYTHONPATH, ici on l'ajoute au chemin d'import. +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[3] / "libs")) + import os import sys from pathlib import Path diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index ed5d1c9..41bac1d 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -12,7 +12,8 @@ services: crawler: build: - context: apps/crawler + context: . + dockerfile: apps/crawler/Dockerfile environment: DATABASE_URL: ${DATABASE_URL} FLARESOLVERR_URL: http://flaresolverr:8191 @@ -29,7 +30,8 @@ services: geocoder-enqueue: build: - context: apps/geocoder + context: . + dockerfile: apps/geocoder/Dockerfile working_dir: /app environment: DATABASE_URL: ${DATABASE_URL} @@ -45,7 +47,8 @@ services: geocoder-worker: build: - context: apps/geocoder + context: . + dockerfile: apps/geocoder/Dockerfile working_dir: /app environment: DATABASE_URL: ${DATABASE_URL} @@ -64,7 +67,8 @@ services: groq-worker: build: - context: apps/geocoder + context: . + dockerfile: apps/geocoder/Dockerfile working_dir: /app environment: DATABASE_URL: ${DATABASE_URL} diff --git a/docker-compose.yml b/docker-compose.yml index ac6f215..955c9b3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,8 @@ services: geocoder-enqueue: build: - context: apps/geocoder + context: . + dockerfile: apps/geocoder/Dockerfile working_dir: /app environment: DATABASE_URL: ${DATABASE_URL} @@ -17,7 +18,8 @@ services: geocoder-worker: build: - context: apps/geocoder + context: . + dockerfile: apps/geocoder/Dockerfile working_dir: /app environment: DATABASE_URL: ${DATABASE_URL} @@ -36,7 +38,8 @@ services: groq-worker: build: - context: apps/geocoder + context: . + dockerfile: apps/geocoder/Dockerfile working_dir: /app environment: DATABASE_URL: ${DATABASE_URL} diff --git a/apps/crawler/src/health.py b/libs/bm_health.py similarity index 90% rename from apps/crawler/src/health.py rename to libs/bm_health.py index 0947e81..e861ad5 100644 --- a/apps/crawler/src/health.py +++ b/libs/bm_health.py @@ -1,5 +1,13 @@ """ -Contrôle de santé périodique des services de fond. +Contrôle de santé périodique des services de fond — module PARTAGÉ. + +Vit dans libs/ et non dans une app : le crawler et le geocoder en avaient +deux copies rigoureusement identiques, dont une seule était couverte par des +tests. Rien ne signalait une divergence, et une correction appliquée d'un +seul côté serait passée inaperçue. + +Embarqué dans les images par les Dockerfile (contexte de build = racine du +dépôt) et rendu importable en local par les conftest.py de chaque app. Ces services ne sont pas exposés par Traefik : aucune sonde HTTP externe ne peut les atteindre. Sans ce module, un crawler dont FlareSolverr est injoignable ou