Projet

Général

Profil

Development #26836

api santé : fraicheur des données

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

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

0%

Temps estimé:
Patch proposed:
Oui
Planning:

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

Révision d5984fa6 (diff)
Ajouté par Frédéric Péters il y a plus de 5 ans

add caching to health API (#26836)

Révision 78782f85 (diff)
Ajouté par Frédéric Péters il y a plus de 5 ans

views: add import misplaced in #26761 (#26836)

Historique

#1

Mis à jour par Christophe Siraut il y a plus de 5 ans

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)?

#2

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.

#3

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)

#4

Mis à jour par Christophe Siraut il y a plus de 5 ans

Avec le cache paramétrable par élément.

#5

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).

#7

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.

#8

Mis à jour par Christophe Siraut il y a plus de 5 ans

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.

#9

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

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.

#10

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.

#11

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

Voilà avec le retour à l'URL précédente, ouverte.

#12

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.

#13

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)
#14

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

Formats disponibles : Atom PDF