Projet

Général

Profil

Development #46713

horaire de diffusion des rappels

Ajouté par Frédéric Péters il y a plus de 3 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
16 septembre 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Pour le moment,

        # 12 hours time window to run the command and send reminder, thus excluding old events
        starts_after = timezone.now() + reminder_delta - timedelta(hours=12)
...
        bookings = Booking.objects.filter(
...
            event__start_datetime__gte=starts_after,

mais il me semblerait plus opportun de se baser uniquement sur le jour, ignorer l'heure; qu'un rendez-vous soit le lendemain à 9h, ou le lendemain à 17h, avoir le rappel envoyé au même moment, et que ce moment soit une heure décente; le - timedelta(hours=12) me donne l'impression qu'un SMS va être envoyé à 4h du matin pour un rendez-vous à 16h, et je n'ai pas cherché d'étude là-dessus mais je dirais que ça marche moins bien comme horaire (genre ça se perd dans le nettoyage des notifs de la nuit).

Bref, je dirais au doigt mouillé qu'il faudrait concentrer l'envoi des rappels entre 13h et 17h.


Fichiers

Révisions associées

Révision c513f4b1 (diff)
Ajouté par Valentin Deniaud il y a plus de 3 ans

reminders: adjust and explain sending time (#46713)

Historique

#1

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Frédéric Péters a écrit :

le - timedelta(hours=12) me donne l'impression qu'un SMS va être envoyé à 4h du matin pour un rendez-vous à 16h

Nop, on envoie les rappels pour les évènements entre [dans x jours - 12h, dans x jours]¹, or le cron tournant toutes les heures, on envoie normalement que dans x jours pile, [dans x jours - 12h, dans x jours - 1h] étant vide. Sauf en cas d'erreur où il faudra pouvoir réessayer au cron suivant.

Donc les rappels sont envoyés pile x jours avant, donc si le rdv n'est pas à une heure étrange, le rappel non plus.

Peut-être que réessayer 12 fois c'est néanmoins trop, et que dans le cas où quelqu'un configure ça à l'arrache sur un évènement qui a déjà des réservation et déjà dans l'intervalle ci-dessus des rappels vont être envoyés à des heures pas chrétiennes. Donc à la limite, passer à genre 5h.

Bref, je dirais au doigt mouillé qu'il faudrait concentrer l'envoi des rappels entre 13h et 17h.

Pourquoi pas, mais le comportement ci-dessus a quand même l'avantage d'étaler automatiquement les rappels sur la journée. Là il faut ajouter la logique d'étalage des rappels (ou pas mais sans logique d'étalage on augmente le risque de perdre des messages, par ex le comportement du connecteur OVH en cas d'envoi massif actuellement n'est pas terrible il me semble).

¹ Pour être encore plus explicite, parce que ça me fait des nœuds au cerveau ces histoires :
  • Évènement le 03/01 à 14h, rappel un jour avant = envoi le 02/01 à 14h
  • Le cron tourne, à H-1 de la première tentative, on regarde les évènements dans [02/01 1h, 02/01 13h]
  • Ensuite [02/01 2h, 02/01 14h], bingo l'évènement est dedans
  • Si l'envoi foire, au prochain run on regarde [02/01 3h, 02/01 15h], l'évènement est toujours dedans donc on retente l'envoi
#2

Mis à jour par Frédéric Péters il y a plus de 3 ans

¹ Pour être encore plus explicite, parce que ça me fait des nœuds au cerveau ces histoires :

Il y avait un peu de ça chez moi; j'ai trouvé hier le code difficile à comprendre, peut-être que ça appelle juste un plus long commentaire sur la logique cherchée.

Et si je comprends bien, l'envoi limité à l'après-midi se ferait simplement en zappant le job avant midi; genre if timezone.now().hour < 12: return au début; et le seul truc que ça change c'est le nombre de tentatives en cas d'erreur.

par ex le comportement du connecteur OVH en cas d'envoi massif actuellement n'est pas terrible il me semble).

La logique de réenvoi doit être portée par le connecteur SMS et je pense que c'est déjà le cas; ici on se protège juste d'un problème genre passerelle down.

#3

Mis à jour par Valentin Deniaud il y a plus de 3 ans

  • Assigné à mis à Valentin Deniaud
#4

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Frédéric Péters a écrit :

peut-être que ça appelle juste un plus long commentaire sur la logique cherchée.

Oui et en fait là le commentaire qui parle de ne pas regarder les vieux évènements détourne complètement du vrai truc à comprendre, je change ça.

Et si je comprends bien, l'envoi limité à l'après-midi se ferait simplement en zappant le job avant midi; genre if timezone.now().hour < 12: return au début; et le seul truc que ça change c'est le nombre de tentatives en cas d'erreur.

Ça fait varier le nombre de tentative en fonction de l'heure de l'évènement, ça ne me paraît pas top. Et dans un cas extrême genre un évènement à minuit, plus qu'une tentative.

En l'absence d'étude comportementale sur la lecture des SMS entre 9h du mat' et midi donc, je préférerais rester sur le fonctionnement actuel qui d'après les tests envoie les messages à la bonne heure, et qui a le bonus d'un fonctionnement par petites salves espacées et sympathiques, à peu de frais.

Je réduis quand même la fenêtre, 12h c'est beaucoup, mettons 6.

#5

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Solution proposée à Résolu (à déployer)

Poussé, après avoir réduit la longeur des lignes de commentaire vu que black autorise ça et que c'est plus lisible.

commit c513f4b16b7f07c683a3d4e2455b87276f27f0be
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Thu Sep 17 13:57:49 2020 +0200

    reminders: adjust and explain sending time (#46713)
#6

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF