betagouv / rdv-service-public

Prise de RDV pour les services publics
https://rdv.anct.gouv.fr
GNU Affero General Public License v3.0
12 stars 2 forks source link

ne plus dépendre de la db à l’initialisation : méthode Rdv#hardcoded_attribute_names #4394

Closed adipasquale closed 1 day ago

adipasquale commented 3 days ago

Contexte

Lors du downtime d’hier soir on s’est rendus compte que les serveurs web ne démarrent pas lorsque la db est inaccessible. C’est un peu problématique, notamment car pour publier une page de maintenance via le mécanisme de Scalingo il faut avoir des workers que l’on peut démarrer (c’est un peu bizarre).

En règle générale ça paraît être une bonne idée que les process web puissent démarrer sans connexion db.

Description du blocage

Le souci identifié vient des form concerns comme Rdv Form Concern qui appellent Rdv.attribute_names qui fait un appel à la db.

Solution proposée

Je n’ai pas trouvé de solution élégante pour récupérer les noms d’attributs depuis le config/schema.rb sans appeler la DB.

Je propose ici d’hardcoder ces attributs dans une nouvelle méthode Rdv.hardcoded_attribute_names.

Je mets en place un test unitaire qui nous avertira lorsque ces attributs hardcodés divergent de la réalité de la db, par ex suite à une migration sur rdvs

J’essaie aussi de mettre en place un test dans la CI pour s’assurer que les processes web démarrent bien sans db (ni redis). ça met 17s et c’est fait en parallèle des autres actions donc a priori ça ne ralentit pas le build. mais c’est discutable et un poil bancal donc on peut supprimer cette partie de cette PR sans souci

adipasquale commented 3 days ago

merci François ! je suis un peu plus à l’aise avec cette direction de permettre le démarrage de l’app sans db qu’avec le rajout de la rack app de maintenance (en revannche je pense qu’il faudra récupérer le fichier maintenance.html ici aussi).

Ça me parait plus robuste et une bonne pratique d’avoir des serveurs web qui peut s’initialiser uniquement avec du code et sans dépendances sur des services externes. Ça permet de pouvoir servir les pages statiques (est-ce que le cache casserait le rendu lorsqu’il n’y a pas de db ? je ne sais pas)

La rack app de maintenace m’inquiète un peu en termes de sécurité mais c’est sûrement infondé.

Je n’ai pas un avis très fort, donc si toi c’est le cas on peut partir sur la rack app ça me va aussi ! en tout cas je n’ai pas très envie qu’on se retrouve dans la même situation où on ne peut pas mettre de page de maintenance correcte au prochain downtime de la db

victormours commented 2 days ago

@adipasquale Pour info il y a eu ces discussions sur la mise à jour des pages d'erreur : Pad : https://pad.numerique.gouv.fr/1520X3NOR069yF5fZjJkPA Maquettes : https://www.figma.com/design/1IITBeGk9MuYSOhBW3a8iv/Pages-statiques?node-id=1-2&t=1ec85PjEre4WtqKu-0 Thread : https://mattermost.incubateur.net/betagouv/pl/xgi5our673gxzkg1bzq4d1uw3y

adipasquale commented 2 days ago

petit ping @francois-ferrandis qu’en penses-tu?

francois-ferrandis commented 2 days ago

petit ping @francois-ferrandis qu’en penses-tu?

Un peu inquiet de la complexité ajoutée, mais c'est vrai que c'est rassurant de pouvoir charger le code sans avoir besoin d'accès à PG.

Je n'ai pas d"objection, ça me va bien de merger. :heavy_check_mark: