sms: update credit left in check_status (#79444) #316

vdeniaud · 2023-07-06T11:29:46+02:00

vdeniaud commented

2023-07-06 11:29:46 +02:00

Ça fait revenir sur le tout récent #78939.

vdeniaud force-pushed wip/79444-sms-ovh-marquer-le-connecteur-do from 9dd66b0e79 to 5cda735517

2023-07-06 11:46:21 +02:00

Compare

vdeniaud changed title from ~~WIP: sms: update credit left in check_status (#79444)~~ to sms: update credit left in check_status (#79444)

2023-07-06 11:46:39 +02:00

fpeters commented

2023-07-06 11:55:29 +02:00

Ça fait revenir sur le tout récent #78939.

Il me semble qu'il y aurait possibilité sans revenir là-dessus ("pour taire les sentry"); plutôt que se baser sur la mise en down parce qu'une exception est levée lors de la mise à jour des crédits (et également risquer l'envoi d'un mail toutes les 5 minutes ?), avoir le code de check_status() limité à l'appel http et y attraper l'exception et utiliser self.set_availability_status('down', message="...") pour marquer down.

> Ça fait revenir sur le tout récent #78939. Il me semble qu'il y aurait possibilité sans revenir là-dessus ("pour taire les sentry"); plutôt que se baser sur la mise en down parce qu'une exception est levée lors de la mise à jour des crédits (et également risquer l'envoi d'un mail toutes les 5 minutes ?), avoir le code de check_status() limité à l'appel http et y attraper l'exception et utiliser `self.set_availability_status('down', message="...")` pour marquer down.

vdeniaud commented

2023-07-06 12:13:19 +02:00

Il me semble qu'il y aurait possibilité sans revenir là-dessus ("pour taire les sentry");

Je ne pensais pas revenir là dessus, j'ai extrapolé que les exceptions levées dans check_status étaient rattrapées, donc pas de sentry. Si ce n'est pas le cas effectivement le patch n'est pas bon.

et également risquer l'envoi d'un mail toutes les 5 minutes ?

Non ça c'est géré dans la méthode, maximum un mail par jour (pour déjà ne pas en envoyer toutes les heures).

avoir le code de check_status() limité à l'appel http et y attraper l'exception et utiliser self.set_availability_status('down', message="...") pour marquer down.

Dac je peux aussi faire comme ça, c'est moins DRY mais plus explicite.

> Il me semble qu'il y aurait possibilité sans revenir là-dessus ("pour taire les sentry"); Je ne pensais pas revenir là dessus, j'ai extrapolé que les exceptions levées dans check_status étaient rattrapées, donc pas de sentry. Si ce n'est pas le cas effectivement le patch n'est pas bon. > et également risquer l'envoi d'un mail toutes les 5 minutes ? Non ça c'est géré dans la méthode, maximum un mail par jour (pour déjà ne pas en envoyer toutes les heures). > avoir le code de check_status() limité à l'appel http et y attraper l'exception et utiliser `self.set_availability_status('down', message="...")` pour marquer down. Dac je peux aussi faire comme ça, c'est moins DRY mais plus explicite.

fpeters commented

2023-07-06 13:00:38 +02:00

Il me semble qu'il y aurait possibilité sans revenir là-dessus ("pour taire les sentry");

Je ne pensais pas revenir là dessus, j'ai extrapolé que les exceptions levées dans check_status étaient rattrapées, donc pas de sentry. Si ce n'est pas le cas effectivement le patch n'est pas bon.

Je n'ai pas vérifié, je voulais juste noter qu'il ne fallait pas totalement revenir à ce qui se faisait avant l'autre ticket.

et également risquer l'envoi d'un mail toutes les 5 minutes ?

Non ça c'est géré dans la méthode, maximum un mail par jour (pour déjà ne pas en envoyer toutes les heures).

ok top.

avoir le code de check_status() limité à l'appel http et y attraper l'exception et utiliser self.set_availability_status('down', message="...") pour marquer down.

Dac je peux aussi faire comme ça, c'est moins DRY mais plus explicite.

S'il n'y a pas de trace sentry sur le check_status c'est ok; je n'avais pas relu https://git.entrouvert.org/entrouvert/passerelle/pulls/295/files donc si je comprends bien le chemin après ce ticket ça aura été :

des traces toutes les heures via le hourly,
plus de traces grâce à l'except,
le hourly remplacé par le check_status, avec du coup plus besoin du try/except précédent vu que c'est attrapé lors du check,

et ça me va ainsi.

> > Il me semble qu'il y aurait possibilité sans revenir là-dessus ("pour taire les sentry"); > > Je ne pensais pas revenir là dessus, j'ai extrapolé que les exceptions levées dans check_status étaient rattrapées, donc pas de sentry. Si ce n'est pas le cas effectivement le patch n'est pas bon. Je n'ai pas vérifié, je voulais juste noter qu'il ne fallait pas totalement revenir à ce qui se faisait avant l'autre ticket. > > et également risquer l'envoi d'un mail toutes les 5 minutes ? > > Non ça c'est géré dans la méthode, maximum un mail par jour (pour déjà ne pas en envoyer toutes les heures). ok top. > > avoir le code de check_status() limité à l'appel http et y attraper l'exception et utiliser `self.set_availability_status('down', message="...")` pour marquer down. > > Dac je peux aussi faire comme ça, c'est moins DRY mais plus explicite. S'il n'y a pas de trace sentry sur le check_status c'est ok; je n'avais pas relu https://git.entrouvert.org/entrouvert/passerelle/pulls/295/files donc si je comprends bien le chemin après ce ticket ça aura été : * des traces toutes les heures via le hourly, * plus de traces grâce à l'except, * le hourly remplacé par le check_status, avec du coup plus besoin du try/except précédent vu que c'est attrapé lors du check, et ça me va ainsi.

vdeniaud commented

2023-07-06 14:10:15 +02:00

avec du coup plus besoin du try/except précédent vu que c'est attrapé lors du check,

Oui j'ai lu le code (BaseResource.availability) et je confirme que c'est le cas.

> avec du coup plus besoin du try/except précédent vu que c'est attrapé lors du check, Oui j'ai lu le code (BaseResource.availability) et je confirme que c'est le cas.

ecazenave approved these changes 2023-07-06 14:29:07 +02:00

fpeters approved these changes 2023-07-06 14:29:08 +02:00

vdeniaud merged commit 5cda735517 into main

2023-07-06 15:16:37 +02:00

vdeniaud deleted branch wip/79444-sms-ovh-marquer-le-connecteur-do

2023-07-06 15:16:38 +02:00

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/passerelle#316