Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permet le changement de titre d'un contenu publié entre deux passages du watchdog de publication #6269

Merged
merged 3 commits into from
Apr 26, 2022

Conversation

philippemilink
Copy link
Member

@philippemilink philippemilink commented Mar 25, 2022

Corrige un bug qui faisait planter le watchdog de publication, si entre deux exécutions du watchdog un contenu était publié, puis son titre était changé et republié avec ce nouveau titre.

Le problème vient, entre autres, de l'affichage du titre du contenu dans les messages de logs. Le titre étant récupéré depuis le manifest, celui-ci est indiqué comme étant dans un dossier qui n'existe plus, puisque le contenu a été renommé (et donc aussi le dossier contenant le manifest).

Cette PR corrige problème de plusieurs façons :

  • si on n'arrive pas à récupérer le titre depuis le manifest, on utilise celui qu'on a en base de données
  • lors de la publication, on supprime les demandes de publications au watchdog qui n'ont pas été encore traitées, pour ne demander la publication que de la version actuelle contenu

J'en ai profité aussi pour:

  • ajouter le test correspondant à ce cas
  • ajouter une option --once au publication_watchdog pour ne pas le faire attendre de nouvelles demandes de publication (nécessaire pour le test)
  • enlever du code mort est-il complètement mort ? cf Retire l'export de contenu au format HTML #5997

Le risque de race condition entre la demande de publication et le traitement par le watchdog existe toujours (le contenu avec le nouveau titre est publié pendant que le watchdog traite la demande précédente correspondant à la version précédente du contenu), mais vraiment réduite et la gestion des erreurs dans le watchdog est bien meilleure maintenant (ça ne devrait pas le faire planter en boucle, même après avoir redémarré pour cause d'erreur).

Je ne suis pas complètement satisfait de cette correction, j'ai l'impression d'appliquer des bouts de rustines plutôt que de corriger la vraie source du problème...

En attente de #6264, mais les premières revues de codes sont d'ores et déjà les bienvenues.

Contrôle qualité

  • make zmd-start
  • Lancer le watchdog une première fois : python3 manage.py publication_watchdog --once. Tous les contenus générés par les fixtures, en attente de publication, sont publiés. Les logs rapportent des erreurs dans les publications, mais ce n'est pas grave. La commande finit par s'arrêter et rend la main.
  • Si on relance le watchdog de la même manière, rien ne se passe, la commande rend la main
  • Si on relance le watchdog sans l'option --once, le watchdog se met à attendre (Ctrl+C pour l'interrompre)
  • En ayant arrêté le watchdog :
    1. Lancer le site
    2. Se connecter
    3. Créer un billet
    4. Le publier
    5. Éditer le billet et changer son titre
    6. Publier le billet
    7. Lancer le watchdog : il n'y a que 4 publications (pour les 4 formats qu'on gère), et il n'y a pas d'exception qui sont générées.

@philippemilink philippemilink added this to En développement in Suivi des PR via automation Mar 25, 2022
@philippemilink philippemilink force-pushed the watchdog-contenu-renomme branch 2 times, most recently from 4df3e25 to 14846e8 Compare March 25, 2022 16:40
@coveralls
Copy link

coveralls commented Mar 25, 2022

Coverage Status

Coverage increased (+0.2%) to 87.45% when pulling 0afa2b1 on philippemilink:watchdog-contenu-renomme into 9ddac71 on zestedesavoir:dev.

Suivi des PR automation moved this from En attente de QA to Fusionnable après rebase Apr 26, 2022
Copy link
Member

@Situphen Situphen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je ne suis pas complètement satisfait de cette correction, j'ai l'impression d'appliquer des bouts de rustines plutôt que de corriger la vraie source du problème...

Je suis assez d'accord mais je ne vois pas ce qu'on pourrait faire de mieux donc pour l'instant ça fera l'affaire.

@Situphen Situphen enabled auto-merge (rebase) April 26, 2022 14:00
@Situphen Situphen merged commit fdac957 into zestedesavoir:dev Apr 26, 2022
Suivi des PR automation moved this from Fusionnable après rebase to Fusionnée Apr 26, 2022
@philippemilink philippemilink deleted the watchdog-contenu-renomme branch May 4, 2022 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Suivi des PR
  
Fusionnée
Development

Successfully merging this pull request may close these issues.

None yet

3 participants