db : le cycle de vie d'une recommandation entre en base (#164) #180
No reviewers
Labels
No labels
Compat/Breaking
EC01
EC02
EC03
EC04
EC05
EC06
Kind/BDD
Kind/Back
Kind/Bug
Kind/CICD
Kind/Cloud
Kind/Contenu
Kind/Data
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Front
Kind/IA
Kind/Infra
Kind/Monitoring
Kind/Security
Kind/Testing
Portée/Post-jury
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
ops/alerte
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
g2/enervision!180
Loading…
Reference in a new issue
No description provided.
Delete branch "olivier/164-recommandation-cycle"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Ferme #164.
cycle.rapprocher, écrit au #154, attendait de savoir où écrire : voici la table.Le numéro n'est pas celui du ticket
Le #164 demande une
0017_zone_or_recommandation_cycle.sql. 0017 est prise par le #36 (référence de comparaison des prévisions) et 0018 par le #168 (libellé et état des alertes), toutes deux fusionnées dansdeveloppendant que le #164 attendait. Le fichier est donc la 0019, même contenu, même intention. La 0018 annonçait d'ailleurs le rendez-vous : «resolue_avient avec le cycle de vie des recommandations au #164 ».Ce que la migration fait
Trois colonnes sur
public.recommandation, et la 0014 n'est pas retouchée :statuttext not null default 'active'active/resolue. Le défaut vaut aussi pour les lignes déjà chargées : la 0014 n'enregistre que des émissionsresolue_atimestamptznullablealerteau #168, la colonne arrive avec ce qui la remplit —cycle.Resolutionle produit déjàactiontext not nulllibelleporte le constat et laisse l'exploitant sans quoi faireL'unicité de la 0014 tombe.
unique (site_id, horodatage, regle_id, regle_version)laissait coexister deux lignes actives pour le même site et la même règle — il suffit que l'horodatage diffère, c'est-à-dire à chaque heure — et faisait de la version une part de l'identité, alors que le #153 pose l'inverse : « recaler un seuil ne doit ni fermer ni rouvrir ce qui est en cours ». À sa place, un index unique partiel(site_id, regle_id) where statut = 'active'. Partiel, parce que l'historique des résolues doit pouvoir compter plusieurs lignes pour le même couple.Le rejeu idempotent d'une journée que l'ancienne contrainte servait est perdu, et c'est assumé : il appartient désormais au job, pas au schéma.
Le critère 5, tranché :
default ''puisdrop defaultUn
add column ... not nullsans défaut réussit sur une table vide et échoue dès la première ligne présente. Les migrations s'appliquent au démarrage du serveur, donc à un moment que personne ne choisit.La table est vide partout aujourd'hui — rien n'y écrit : le chargement de la zone or ne remplit que
mesure,qualite_jouretalerte, et l'API sert des fixtures. Mais une migration qui ne marche que sur une table vide est une migration qui tombe le jour où elle ne l'est plus. D'où les deux gestes : le défaut rend l'ajout sûr quelle que soit l'histoire de la base, son retrait fait qu'une insertion qui oublie l'action échoue au lieu d'écrire une consigne vide.Le nullable a été écarté pour la raison qui vaut déjà côté moteur, où
regle.pyrefuse d'émettre : « une recommandation qui ne dit pas quoi faire n'est pas une recommandation ».Une contrainte de plus que le ticket ne demandait
check ((statut = 'resolue') = (resolue_a is not null)). L'état et son instant ne peuvent pas se contredire — c'est l'invariant pour lequelresolue_aexiste. Signalé ici parce que le #164 ne l'énumérait pas.Éprouvé sur PostgreSQL, pas seulement lu
Deux bases d'essai jetables sur
ev-postgres, supprimées à la fin ; nienervision_preprodnienervision_prodtouchées.statut = active,resolue_anul,action = ''— le cas exact du critère 5 ;NOTICE, aucune erreur ;recommandation_site_id_horodatage_regle_id_regle_version_key— ce qui prouve au passage que le nom écrit dans ledrop constraintest bien celui que PostgreSQL fabrique. La 0019 se rejoue ensuite par-dessus.Les gardes ont été poussées, et non supposées :
recommandation_activeactionnot nullstatutinconnurecommandation_statutresolue_a, ou active avec unresolue_arecommandation_resolue_aCôté tests
tests/unit/db/test_migration_recommandation_cycle.py, 14 cas. Dont une garde de dérive avec le moteur : la 0019 est le « où écrire » quecycle.rapprocherattend, et rien d'autre ne surveille que les deux se parlent des mêmes champs — le job n'existe pas encore. Et une garde sur le nom de la contrainte retirée, recomposé depuis la clause de la 0014 plutôt que recopié : le jour où quelqu'un touche à cette clause, ledrop constraint if existsne trouverait plus rien et ne le dirait pas.NOUVEAUXdetest_migrations_zone_or.py(critère 4) plutôt que de recopier les deux garde-fous génériques — sens de retour écrit, rejeu sans effet de bord.pytest tests/unit: 932 passés, 1 sauté.ruff checketruff format --checkpropres.Hors périmètre, comme le ticket le pose
Brancher
cycle.rapprochersur cette persistance — lecture des actives, écriture des ouvertures et des résolutions depuis le servicerecommendations— reste à faire dans un ticket séparé. Ni changement d'API, ni de tableau de bord.Approuvée. La migration est juste et l'index unique partiel est ce que le #153 demande : l'ancienne unicité laissait coexister une ligne active par heure pour le même site et la même règle, et faisait de la version une part de l'identité alors que le ticket pose l'inverse.
J'ai vérifié le nom de la contrainte en base plutôt que de le déduire, parce qu'un
drop constraint if existsqui ne trouve rien ne dit rien :Il tombe juste. Le
default ''puisdrop defaultest le bon geste, et ton commentaire dit pourquoi mieux que je ne l'aurais fait.Deux points bloquants, et ils sont chez moi
Ta migration casse la #175, ouverte en parallèle. Ce n'est pas un reproche : c'est à elle de s'aligner, pas à toi de reculer.
Le
on conflictde la #175 visait la contrainte que tu supprimes. PostgreSQL rend 42P10, l'insertion entière échoue, à chaque passe horaire, et ça ne se voit que dansrecommandations.log.actiondevientnot nullsans défaut, et la #175 ne l'écrivait pas. C'est exactement ce que ton commentaire annonce vouloir : « une insertion future qui oublie l'action échoue au lieu d'écrire une consigne vide ». Elle a attrapé la mienne.Les deux sont corrigés dans la #175, qui prend ta migration dans sa branche pour pouvoir s'y croiser. Elle en profite pour lire
statutau lieu d'une fenêtre de 26 heures, et pour ÉCRIRE les résolutions au lieu de les compter : sansstatut,a_resoudrepartait à la poubelle et une recommandation restait ouverte pour toujours.Une correction factuelle
Ta demande dit « la table est vide partout aujourd'hui ». Elle contient une ligne depuis que le job de la #175 a tourné sur le serveur ce matin :
Ça ne change rien à ta migration, le
default ''la traite correctement, et c'est bien pour ça qu'il est là. Mais le critère 5 n'est plus théorique : il s'applique pour de vrai au prochain déploiement.Ordre de fusion
Celle-ci d'abord, la #175 ensuite. La #175 référence
statutetaction: fusionnée avant, elle casserait sur des colonnes absentes.