Development #26836
api santé : fraicheur des données
0%
Description
Actuellement l'API santé fait en live ses vérifications, comme ça demande des appels réseau etc. ça rend toute l'affaire bien lente.
Voir pour mettre du cache, et/ou calculer ça via un job cron, etc.
Fichiers
Révisions associées
Historique
Mis à jour par Christophe Siraut il y a plus de 5 ans
- Fichier 0001-health-api-cache-results-26836.patch 0001-health-api-cache-results-26836.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Proposition de mise en cache simple, j'ai mis 10 minutes par défaut, sans y réfléchir beaucoup.
L'exception sur HEALTHAPI_TIMEOUT est moche, est-ce que je peux poser ce paramètre dans le settings.py principal (ou ailleurs, ou autre solution)?
Mis à jour par Frédéric Péters il y a plus de 5 ans
L'exception sur HEALTHAPI_TIMEOUT est moche, est-ce que je peux poser ce paramètre dans le settings.py principal (ou ailleurs, ou autre solution)?
Je ne vois pas ce qui l'empêcherait.
Cela étant, je ne suis pas favorable à un cache aussi large, je préférerais que soient visés de manière plus précise les différents éléments (resolvable, valid certificate, etc.), histoire de pouvoir garder une vue au plus près possible du moment, sans la charger constamment de vérifications sur des trucs qui bougent très lentement.
Mis à jour par Thomas Noël il y a plus de 5 ans
Et le code ainsi fait n'est pas multi-tenant (les settings doivent passer par "from django.conf import settings" et être lu dans les vues)
Mis à jour par Christophe Siraut il y a plus de 5 ans
- Fichier 0001-cache-health-checks-results-26836.patch 0001-cache-health-checks-results-26836.patch ajouté
Avec le cache paramétrable par élément.
Mis à jour par Frédéric Péters il y a plus de 5 ans
Je lirai davantage plus tard mais au niveau de la forme, jamais return(val)
(return n'est pas une fonction).
Mis à jour par Frédéric Péters il y a plus de 5 ans
Testé en local, ça fonctionne mais,
self.has_certificate = self.cache(service.has_valid_certificate, 3600)
change l'API. Ça devrait être has_valid_certificate.
Mis à jour par Christophe Siraut il y a plus de 5 ans
- Fichier 0001-cache-health-checks-results-26836.patch 0001-cache-health-checks-results-26836.patch ajouté
rebasé, corrigé les 2 erreurs mentionnées ici, et supprimé self.base_url qui n'est pas utilisé. Je n'ai pas encore retesté, je le ferai sur mon vrai ordi.
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Fichier 0001-add-caching-to-health-API-26836.patch 0001-add-caching-to-health-API-26836.patch ajouté
J'avais raté la mise à jour du patch, désolé.
Du coup j'étais parti pour corriger les tests, puis je me suis trouvé à faire une puis deux puis trois modifications, me rendre compte que hobo/rest/ se vidait de tout, continuer sur ma lancée.
Avec comme résultat de raccourcir pas mal le tout, avec au niveau de l'objet service directement une méthode récupérant les différentes propriétés intéressantes (plutôt que passer par une sérialiseur djangorestframework), avec l'URL changée pour être sous /sites/, pour bénéficier de la vérification login_required existante plutôt que réintroduire du djangorestframework, et.
Côté fonctionnalité, j'ajoute surtout une variation de la durée de cache, pour diminuer les moments où les propriétés se trouvent expirées au même moment.
Côté tests, il passent désormais et j'y ajoute une vérification que le cache est bien effectif.
Côté bugs, ou fonctionnalités, je ne suis pas arrivé à me décider, ton patch ne mettait pas en cache un résultat négatif, j'ai de mon côté opté pour la mise en cache.
Mis à jour par Frédéric Péters il y a plus de 5 ans
bénéficier de la vérification login_required existante
Ce qui est une erreur dans la mesure où l'objectif de cette vue doit dépasser l'affichage des infos sur la page d'accueil.
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Fichier 0001-add-caching-to-health-API-26836.patch 0001-add-caching-to-health-API-26836.patch ajouté
Voilà avec le retour à l'URL précédente, ouverte.
Mis à jour par Christophe Siraut il y a plus de 5 ans
- Statut changé de Solution proposée à Solution validée
Ok pour moi. Notez que je n'ai pas beaucoup réfléchi à la pertinence de la durée des cache mais cela pourra facilement être changé si nécessaire.
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit d5984fa6c3b10dd3c2aef6579142cad4cb466679 Author: Frédéric Péters <fpeters@entrouvert.com> Date: Wed Nov 28 19:59:45 2018 +0100 add caching to health API (#26836)
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Statut changé de Résolu (à déployer) à Solution déployée
add caching to health API (#26836)