Marmotte/respit feature by marmotteabbas · Pull Request #1415 · EsupPortail/Esup-Pod

marmotteabbas · 2026-03-18T15:33:37Z

Dev from Grenoble University about the archive and the respit calculator system.
Author : Florent Paccalet
Mail : florent.paccalet@univ-grenoble-alpes.fr

Badatos · 2026-03-18T15:40:55Z

Merci pour cette PR !
Pour que le code soit conforme, je t'invite à d'abord lancer la commande make pystyle pour t'assurer que les fichiers respectent les recommandations flake8 ;)

marmotteabbas · 2026-03-18T15:59:56Z

Très bien.
Je dois aussi intégrer un .env.dev au commit c'est bien ça ?

Badatos · 2026-03-18T16:06:27Z

Très bien. Je dois aussi intégrer un .env.dev au commit c'est bien ça ?

non, pas de .env.dev nécessaire à priori.

Badatos

Voici déjà quelques corrections à apporter aux chaines i18n

Badatos · 2026-03-23T16:30:51Z

+msgstr "Ce service n'est pas accessible."
+
+msgid "Link to the video :"
+msgstr "Lien vers la vidéo :"


En fait, avant chaque double-point (ainsi que pour les point-virgule, point d'exclamation, point d'interrogation), il faut un espace insécable en français. (un espace qui ne sera jamais coupé en fin de ligne).

Sur MacOS par exemple, pour taper un espace insécable, on fait ALT+espace

Sur Windows, ça doit se faire avec le raccourci Alt+255

Badatos · 2026-03-23T16:31:45Z

+msgstr "Ce service n'est pas accessible."
+
+msgid "Link to the video :"
+msgstr "Lien vers la vidéo :"


L'autre solution est de trouver un espace insécable ailleurs dans le fichier, et de le copier/coller ;)

Badatos · 2026-03-23T16:34:38Z

+msgstr "Ce service n'est pas accessible."
+
+msgid "Link to the video :"
+msgstr "Lien vers la vidéo :"


Visuellement, il est quasi impossible de distinguer un espace normal d'un espace insécable. Pour les distinguer plus facilement, je vous invite à activer une option dans votre éditeur de texte qui permet de mieux les visualiser.
Sur l'éditeur VS Code par exemple, cela se fait avec le plugin https://marketplace.visualstudio.com/items?itemName=possan.nbsp-vscode
=> les espaces insécables apparaissent alors sur fond rouge, ce qui permet de les distinguer rapidement.

marmotteabbas · 2026-03-25T09:54:10Z

make pystyle

Je ne parviens pas à lance "make pystyle"

root@pod:/usr/src/app# make pystyle
black . -l 90
make: black: No such file or directory
make: *** [Makefile:86: pystyle] Error 127

A quelle niveau le lancer ? Y'a t'il une procédure particulière ? J'ai lancé l'installation dans le conteneur en faisant "pip install pystyle" au préalable.

Je lance la commande "flake8" seul, j'ai tout un tas d'erreur d'écriture que je peux corriger, ça suffit ?

Merci !

Badatos · 2026-03-25T10:11:59Z

make pystyle

Je ne parviens pas à lance "make pystyle"

root@pod:/usr/src/app# make pystyle black . -l 90 make: black: No such file or directory make: *** [Makefile:86: pystyle] Error 127

A quelle niveau le lancer ? Y'a t'il une procédure particulière ? J'ai lancé l'installation dans le conteneur en faisant "pip install pystyle" au préalable.

Je lance la commande "flake8" seul, j'ai tout un tas d'erreur d'écriture que je peux corriger, ça suffit ?

Merci !

En fait, il faut préalablement avoir installé la commande black :

pip install black

C'est notre faute, c'est vrai qu'on aurait du le mettre dans le requiremets-dev.txt

Tu peux en effet corriger à la main ce que te signale flake8, mais black en corrige une bonne partie tout seul, ce serait bête de tout se coltiner ^^.

LoicBonavent · 2026-03-25T10:26:58Z

Hello,
Merci beaucoup pour tout ce travail.
Voici mes quelques remarques :

répis -> répit

si possible, mettre les commentaires en Anglais également

la version init devrait être la 4.3.0 maintenant

marmotteabbas · 2026-03-25T10:28:45Z

make pystyle

Je ne parviens pas à lance "make pystyle"
root@pod:/usr/src/app# make pystyle black . -l 90 make: black: No such file or directory make: *** [Makefile:86: pystyle] Error 127
A quelle niveau le lancer ? Y'a t'il une procédure particulière ? J'ai lancé l'installation dans le conteneur en faisant "pip install pystyle" au préalable.
Je lance la commande "flake8" seul, j'ai tout un tas d'erreur d'écriture que je peux corriger, ça suffit ?
Merci !

En fait, il faut préalablement avoir installé la commande black :
pip install black
C'est notre faute, c'est vrai qu'on aurait du le mettre dans le requiremets-dev.txt

Tu peux en effet corriger à la main ce que te signale flake8, mais black en corrige une bonne partie tout seul, ce serait bête de tout se coltiner ^^.

Ok je fais ça ! J'avais en effet commencer à tout faire à la main ^^

LoicBonavent · 2026-03-25T10:32:28Z

un petit détail : il me semble qu'il faut un espace après # pour les commentaires

LoicBonavent · 2026-03-25T10:40:56Z

Commentaires avec #, mais pas en début de lignes

Les messages doivent être en Anglais avec utilisation de la traduction _()

LoicBonavent · 2026-03-25T10:44:18Z

+        path = os.path.join(mediaPackage_dir, "")
+        shutil.rmtree(path)
+
+        return "/media/video_package/"+vid.slug+".zip"


Ne faudrait il pas utiliser settings.MEDIA_ROOT plutôt que /media en dur ?

Je n'ai trouvé aucune déclaration dans le settings. Ne sachant pas comment cela serait déclaré par les uns et les autres j'ai préféré assuré pour avoir de manière certaine "/media/video_package/" qui est le chemin que je veux avoir dans tout les cas. De plus une déclaration de MEDIA_ROOT peut influencer tout le logiciel ou une déclaration en dur ici m'assure avec certitude le chemin sans avoir de lien avec le reste pod avec lequel cette brique n'a rien à voir.

Dans ce cas, alors ptet ajouter un parametrage "ARCHIVE_ROOT" que chacun pourra configurer en fonction de son arborescence ?

LoicBonavent · 2026-03-25T10:49:32Z

Pourquoi utiliser des requêtes SQL ?
Je pense qu'il est préférable d'utiliser directement les objets via Video.objects.filter()... par exemple.
J'imagine qu'il doit y avoir un problème de complexité via flake; il faudra sûrement refactoriser.

Plus facile à écrire, infiniment moins de trucs tordus à effectuer pour utiliser SUBSTRING, et oui flake. Je reste la dessus apriori.

@LoicBonavent Est ce que je peux VRAIMENT pas garder les requêtes SQL ? Ca me force à repenser beaucoup de chose, je peux laisser ça comme ça ? Ou partiellement comme ça ?

Hello, le fait de passer par des requêtes SQL pose potentiellement des problèmes de sécurité (typiquement des injections SQL). Mais si c'est géré et que tu es plus à l'aise avec des requêtes SQL, pas de soucis pour ma part.
Peut-être partiellement : dans les cas simples, si c'est possible, utiliser le mapping ORM, sinon des requêtes SQL.

Merci beaucoup !
On y a passé la journée avec Alice mais on a finalement tout enlevé pour passer par du requêtage ORM exclusivement ;)

Cool, bien joué

LoicBonavent · 2026-03-25T10:52:19Z

Attention, il y a du Français dans le code.

LoicBonavent · 2026-03-25T10:53:48Z

Pour les imports, tu peux utiliser l'extension isort; cela te fera des lignes d'import beaucoup plus propres

LoicBonavent · 2026-03-25T11:01:10Z

Il y a du Français, dans le nom des actions.

Pourquoi garder du code commenté ? Ou alors dire s'il y a une utilité dans ce cas.

Manque peut-être des lints

Il faudrait aussi ajouter des docstrings pour les fonctions

Badatos · 2026-03-30T09:27:26Z

Il semble y avoir un souci avec la CI... :/

Container pod-back-with-volumes  Error
 Container pod-back-with-volumes  Error
 Container pod-back-with-volumes  Error
dependency failed to start: container pod-back-with-volumes is unhealthy

Badatos

suite des commentaires

Badatos · 2026-03-25T10:14:33Z

-msgstr "Ce service n'est pas accessible."
+msgstr "Ce service n’est pas disponible."

 msgid "Link to the video :"


Jamais d'espace avant les signes de ponctuation double en anglais (link to the video:)

Badatos · 2026-03-25T10:16:40Z

@@ -11453,15 +11451,15 @@ msgid "Video Archiving"
 msgstr "Archivage Vidéo"

 msgid "Are you sure you want to archive this video ?"


pas d'espace avant les signes de ponctuation doubles en anglais

Badatos · 2026-03-25T14:54:15Z

+        path = os.path.join(mediaPackage_dir, "")
+        shutil.rmtree(path)
+
+        return "/media/video_package/"+vid.slug+".zip"


Dans ce cas, alors ptet ajouter un parametrage "ARCHIVE_ROOT" que chacun pourra configurer en fonction de son arborescence ?

Badatos · 2026-03-30T09:28:44Z

+
+#: pod/video/templates/videos/well_prolonged.html
+msgid "The video life span was extended to"
+msgstr "La vidéo est prolongé jusqu'au"


il faut inclure les variables dans les chaines de caractères

et "prolongéE"

Badatos · 2026-03-30T09:48:18Z

+            <legend>{% trans "Agreement required" %}</legend>
+            <div class="list-group-item">
+                <div class="form-group form-group-required ">
+                    <div>{% trans "Are you sure you want to archive this video ?" %}</div>


pas d'espace avant les signes doubles en anglais

Badatos · 2026-03-30T09:48:38Z

+        </a>
+    </div>
+
+{% endblock page_content %}


ajouter une ligne vide en fin de fichier

comment ça se fait que flake8 ne l'est pas vu c'est un comportement attendu ?
vu sinon

Flake8 ne regarde pas ca spécifiquement, c'est juste recommandé de toujours laisser une ligne vide en fin de fichier pour éviter des conflits sur un ajout ultérieur

Badatos · 2026-03-30T09:48:58Z

+            {% trans "Return to homepage." %}
+        </a>
+    </div>
+{% endblock page_content %}


marmotteabbas · 2026-03-31T13:34:30Z

Il semble y avoir un souci avec la CI... :/

Container pod-back-with-volumes  Error
 Container pod-back-with-volumes  Error
 Container pod-back-with-volumes  Error
dependency failed to start: container pod-back-with-volumes is unhealthy

Je comprend pas trop ce que je dois faire avec tout ça ..

Badatos · 2026-03-31T13:36:34Z

Il semble y avoir un souci avec la CI... :/
Container pod-back-with-volumes  Error
 Container pod-back-with-volumes  Error
 Container pod-back-with-volumes  Error
dependency failed to start: container pod-back-with-volumes is unhealthy
Je comprend pas trop ce que je dois faire avec tout ça ..

Pour le moment, moi non plus. Est-ce que Pod tourne bien chez vous sur cette branche, y'a aucune erreur ?

Badatos · 2026-03-31T14:27:30Z

à priori, le bug ne vient pas de la CI, puisqu'elle fonctionne sur les autres PR.
Il doit y a voir un truc dans les fichiers modifié dans cette PR même. Aucune erreur de votre coté en lancant Pod ?

Non aucune erreur en lançant POD dans docker avec la commande make docker-start
Tout baigne ... donc du coup je sais pas ..

Badatos · 2026-03-31T14:52:49Z

J'ai trouvé l'erreur.
Cela vient du from ..custom.settings_local import WARN_DEADLINES ajouté dans le video/views.py
En effet, par défaut ce parametre n'existe pas, et crée une erreur ImportError: cannot import name 'WARN_DEADLINES' from 'pod.custom.settings_local'

Badatos · 2026-03-31T14:55:31Z

J'ai trouvé l'erreur. Cela vient du from ..custom.settings_local import WARN_DEADLINES ajouté dans le video/views.py En effet, par défaut ce parametre n'existe pas, et crée une erreur ImportError: cannot import name 'WARN_DEADLINES' from 'pod.custom.settings_local'

à la place, il faudrait plutot un WARN_DEADLINES = getattr(settings, "WARN_DEADLINES", [60,30,7])

Badatos · 2026-03-31T14:57:44Z

 )

-# from django.contrib.auth.hashers import check_password
+from ..custom.settings_local import WARN_DEADLINES


Ici, il faudrait plutôt WARN_DEADLINES = getattr(settings, "WARN_DEADLINES", [60,30,7]) pour toujours avoir une valeur par défaut.

waaaaaaaaa c'est ça qui bloque ???? C'est ouf !!! Merci je vais retester ça ...

C'est passé mais Y'a juste eu une petite erreur à la fin de l'exec ... je sais pas trop ce que ça veut dire pour l'instant ..
Merci pour le debug en tout cas ça a grave aidé ;)

Badatos · 2026-03-31T15:28:11Z

On avance... cette fois, ce sont 2 erreurs flake8 :

./pod/video/views.py:128:57: E231 missing whitespace after ','
./pod/video/views.py:128:60: E231 missing whitespace after ','

(à priori, un coup de make pystyle devrait le régler tout seul)

Badatos

quelques corrections de textes :

Badatos · 2026-04-01T13:23:30Z

+
+from pod.playlist.models import Playlist
+
+# https://docs.djangoproject.com/fr/6.0/howto/custom-management-commands/   python3 manage.py respit_launcher


la commande python est à retirer ce ce commentaire, et à placer dans le cartouche pydoc au début du fichier

c'est retiré.
Vu

Badatos · 2026-04-01T13:24:17Z

+
+
+class Command(BaseCommand):
+    help = "Closes the specified poll for voting"


à mon avis cette class n'a rien à voir avec un sondage (poll)

Très bonne remarque je sais pas ce que j'ai foutu.
vu

Badatos · 2026-04-08T10:33:48Z

+                          "default_value": false,
+                          "description": {
+                            "en": [
+                              "Détermines if we enable a link send to take a decision about the futur of the concerned video during the reminder campaign sent by check_obsolete_video."


Enables sending a link to make a decision on the future of the video during the recall campaign instantiated by check_obsolete_video.

Badatos

Quelques remarques supplémentaires :

(nb: je n'ai pas encore fini la relecture, mais je préfère envoyer au fur et à mesure pour que tu ai le temps de corriger)

Badatos · 2026-04-09T09:06:02Z

+    """
+    ###########################
+    # Calcul control access
+    ###########################


Je pense que ce commentaire n'est pas utile et peut être retiré.

Badatos · 2026-04-09T09:06:15Z

+    step_date = vid.date_delete - timedelta(days=higher_warn)
+    display_or_not = date.today() >= step_date
+    ###########################
+    ###########################


commentaire à retirer

Badatos · 2026-04-09T09:14:19Z

+        </form>
+        <a href="/video/archive/and/download/{{ video.slug }}/">{% trans "Export this video." %}</a>
+    {% else %}
+        <h1>{% trans "This service is not available." %}</h1>


Ici, je pense qu'il faudrait distinguer 2 cas :

Quand ENABLE_PAGE_OBSO_MAIL == True, le message est en effet "This service is not available."

Mais quand display_or_not == False, le message devrait plutôt être This video is not close to its deletion date. si je comprend bien le cas.

Y'a le cas où on rend le service indisponible involontairement. La fois où il n'est pas dans une période de campagne de rappel OU qu'il a déjà rajouter du temps de répit par la prolongation (même critère).
Dans tout les cas, l'utilisateur n'a pas besoin et de ne doit pas savoir les conditions. C'est notre politique interne côté admin ça ne le regarde pas.

Badatos · 2026-04-09T09:15:10Z

+        <form action="/video/valid/form/respit/{{ video.slug }}/" method="post">
+            {% csrf_token %}
+            {{ form }}
+            <input type="submit" value="Submit">


Attention "Submit" doit être traduit

Badatos · 2026-04-09T10:14:17Z

+{% block page_content %}
+    <div class="list-group">
+        <fieldset>
+            <legend>{% trans "Archive Download" %}</legend>


Combien de temps cette archive sera-t-elle disponible ? Est-elle supprimée automatiquement au bout d'un temps sur le serveur ?

Non pas de suppression c'est une bonne remarque. Je dois mettre en place un cron pour le nettoyage ?

Je pense que tu peux juste proposer un script shell, et indiquer dans la doc qu'il est conseillé de le mettre en cron à telle fréquence.

Badatos · 2026-04-09T10:34:26Z

+    """
+    This function will launch a archive process and say if it has worked or not on an interface.
+    """
+    if able_or_not_respit(slug) is True and ENABLE_PAGE_OBSO_MAIL:


Attention, il manque un "else" ici, car une vue ne peut pas ne rien renvoyer.
Ici, il faudrait faire un return HttpResponseBadRequest() avec un message indiquant la raison

Badatos · 2026-04-09T10:35:05Z

+    """
+    This function will extend a video about RALLONGE_RESPIT_DAYS days and display the new delete_date.
+    """
+    if able_or_not_respit(slug) is True and ENABLE_PAGE_OBSO_MAIL:


Attention, il manque un "else" ici, car une vue ne peut pas ne rien renvoyer.
Ici, il faudrait faire un return HttpResponseBadRequest() avec un message indiquant la raison

Badatos · 2026-04-17T12:57:47Z

Attention, il y a une erreur dans le code :

Traceback (most recent call last):
  File "/home/runner/work/Esup-Pod/Esup-Pod/pod/video/tests/test_obsolescence.py", line 198, in test_delete_obsolete_video
    ) = cmd.get_video_archived_deleted_treatment()
  File "/home/runner/work/Esup-Pod/Esup-Pod/pod/video/management/commands/check_obsolete_videos.py", line 301, in get_video_archived_deleted_treatment
    self.write_in_csv(vid, "deleted")
AttributeError: 'Command' object has no attribute 'write_in_csv'

Badatos · 2026-04-20T10:34:25Z

Encore des soucis de flake8. Pense au make pystyle ;)

marmotteabbas · 2026-04-20T10:45:51Z

Encore des soucis de flake8. Pense au make pystyle ;)

Je suis trop bête j'avais oublié .. C'est corrigé ça devrait passer maintenant.

Badatos

Quelques changement mineurs à appliquer sur les commentaires :

Badatos · 2026-04-20T14:39:59Z

-
 from datetime import date, timedelta

+# from pod.custom.settings_local import ENABLE_PAGE_OBSO_MAIL


ne pas laisser ce commentaire si ce n'est plus utilisé.

Badatos · 2026-04-20T14:43:49Z

+                #                       date_deletion=vid.date_delete
+                #                   )
+                #                   vid_delete.video.add(vid)
+                #                   vid_delete.save()


ne pas conserver ce code s'il n'est plus utilisé.

Badatos · 2026-04-20T14:48:47Z

+                    data_to_add["date_delete"] = p.date_delete
+                    data_to_add["description"] = p.description
+
+                    # Nombre de chaines
