Development #42230
Uniformiser les retours des connecteurs SMS
0%
Description
... en vue de gérer plusieurs tentatives sur l'envoi asynchrone des SMS en cas d'erreurs récupérables.
Les erreurs récupérables seront à priori les erreur de transport HTTP qui lèvent une RequestException, mais pourraient aussi être fonction de la réponse du service.
Par exemple, pour les service qui retourne cette information, on pourrait tenter de renvoyer les SMS qui ne peuvent pas être délivrés, faute de crédit insuffisant sur le compte du client chez le fournisseur de service.
Fichiers
Demandes liées
Historique
Mis à jour par Nicolas Roche il y a presque 4 ans
- Lié à Development #21465: connecteurs SMS : file pour envois asynchrones ajouté
Mis à jour par Nicolas Roche il y a presque 4 ans
- Fichier 0001-sms-raise-on-recoverable-errors-42230.patch 0001-sms-raise-on-recoverable-errors-42230.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Patch qui crée une exception dédiée aux erreurs récupérables.
Mis à jour par Frédéric Péters il y a presque 4 ans
Ça m'a l'air de vouloir introduire un truc générique et je dirais non, particulièrement quand ça part du cas particulier des SMS où les envois vont être asynchrones et où donc jamais il n'y aura un retour d'API avec ça.
Je dirais que l'ordre des choses devrait plutôt être d'avoir l'envoi asynchrone, et une fois ça en place alors on pourra s'interroger sur les possibilités de rejeu.
Mis à jour par Nicolas Roche il y a presque 4 ans
- Lié à Bug #44730: Erreur SMS OVH : Too much requests. Please retry in 3 seconds. ajouté
Mis à jour par Nicolas Roche il y a plus de 2 ans
- Statut changé de Solution proposée à Nouveau
- Assigné à
Nicolas Rochesupprimé
A présent on a l'envoie asynchrone, mais on n'a pas encore été trop embêté par le rejeu.
S'il fallait traiter les erreur réseau, alors les attraper pour lever SkipJob permettrait le rejeu
(à condition de plus traiter cette exception en amont).
ex:
def send_job(self, *args, **kwargs):
try:
self.send_msg(**kwargs)
except requests.RequestException as exc:
raise SkipJob()
Vu l'unique moment où l'on a constaté que le rejeu nous serait utile (https://dev.entrouvert.org/issues/44730#note-12),
il pourrait être utile de gérer une exception générique comme ce fut proposé dans ce ticket.
ex:
error running send_job job (OVH error: {'status': 429, 'message': 'Too much requests.\nPlease retry in 3 seconds.\n'})
Mis à jour par Benjamin Dauvergne il y a plus de 2 ans
On a déjà SkipJob pour ça, c'est le boulot du job de convertir les exceptions quelconques en SkipJob; par contre rien n'empêche de sous-classer APIError dans le connecteur pour pouvoir les différencier plus facilement dans le job; à la rigueur je vois bien une TransportAPIError pour tout ce qui est erreur au niveau TCP, mais là encore au niveau requests il faudrait différencier une erreur SSL qui n'est pas récupérable en général d'une erreur de connection ou de timeout.
Par contre je verrai bien une amélioration au constructeur de SkipJob pour pouvoir écrire SkipJob(delay=timedelta(minutes=10)) pour dire de pas réessayer 10 minutes, plutôt que de faire des calculs de timestamp.
Mis à jour par Nicolas Roche il y a plus de 2 ans
c'est le boulot du job de convertir les exceptions quelconques en SkipJob
Oui, on ne fait plus que l'asynchrone, c'est vrai.
(du coup je vais pouvoir rejeter ce ticket)
Par contre je verrai bien une amélioration au constructeur de SkipJob pour pouvoir écrire SkipJob(delay=timedelta(minutes=10)) pour dire de pas réessayer 10 minutes, plutôt que de faire des calculs de timestamp.
Sur MDEL_DDPACS je vois que SkipJob est utilisée avec le paramètre after_timestamp, qui semble gérer à la fois une date donnée mais également un delta.
raise SkipJob(after_timestamp=3600 * 6)
Mis à jour par Benjamin Dauvergne il y a plus de 2 ans
Nicolas Roche a écrit :
Par contre je verrai bien une amélioration au constructeur de SkipJob pour pouvoir écrire SkipJob(delay=timedelta(minutes=10)) pour dire de pas réessayer 10 minutes, plutôt que de faire des calculs de timestamp.
Sur MDEL_DDPACS je vois que SkipJob est utilisée avec le paramètre after_timestamp, qui semble gérer à la fois une date donnée mais également un delta.
[...]
Ah ben oui c'est déjà géré, par du code que j'ai écrit en plus, formidable.
Mis à jour par Nicolas Roche il y a plus de 2 ans
- Statut changé de Nouveau à Rejeté
c'est le boulot du job de convertir les exceptions quelconques en SkipJob