PnX-SI / GeoNature

Application de saisie et de synthèse des observations faune et flore
GNU General Public License v3.0
101 stars 102 forks source link

Erreur dans les fichiers de configuration plante l'application #2741

Open joelclems opened 1 year ago

joelclems commented 1 year ago

Actuellement, une erreur dans le fichier de configuration mettre l'application flask en arrêt.

Il pourrait être bien de faire en sorte qu'une erreur dans les fichier de config de plante pas l'application flask -l'api pour la config peut renvoyer une erreur et un message Erreur dans la configuration

Usage: dans le cadre d'instance dockerisée, si l'on a un erreur dans la configuration, après quelques tentative de redémarrage, cela va faire planter les images dockers, on a pas la possibilité de corriger la config sans devoir relancer les images. Avec cette solution, un changement dans le fichier de configuration peut relancer l'application (selon comment l'appel de gunicorn est paramétré), et il n'y a plus le besoin de relancer les images docker

bouttier commented 1 year ago

C’est assez habituel pour une application de ne pas démarrer lorsque son fichier de configuration n’est pas correcte. Et c’est assez inhabituel de redémarrer automatiquement une application à la modification de son fichier de configuration. Je mettrai un point de vigilance à ne pas rentrer dans des fonctionnements totalement inhabituels en raison du besoin BRGM où les utilisateurs n’ont pas la main sur le redémarrage de l’application. Notamment, pour les utilisateurs ayant accès au shell, il me semble vraiment inhabituel et piégeux que lorsque l’on lance geonature avec systemctl, celui-ci nous affiche que l’application s’est bien lancé, et qu’on se rende compte qu’il y a une erreur uniquement en se rendant sur l’interface web. De même lorsqu’on lance geonature en dev. Ainsi, si une telle fonctionnalité venait à être implémenté, il me semble important que se comportement soit strictement optionnel (activé via une variable d’environnement par exemple). Par ailleurs, j’ai du mal à évaluer les impacts de cette fonctionnalité dans la mesure où la configuration est chargé lors d’un import. Que se passera-t-il pour tous les fichiers qui importe la config mais où celle-ci est erronée ?

joelclems commented 1 year ago

Effectivement cela pose la question d'une telle fonctionalité.

J'ai pas d'idée trop précise sur l'implémentation cela pourrait être (en utilisant la variable d'environnement GEONATURE_SAFE_MODE_ON_CONFIG_FAILURE (qui dit mieux)

dans config.py de remplacer cette ligne par https://github.com/PnX-SI/GeoNature/blob/7ee68c5555ecd4e3d1b252dd78254a492cd2d7eb/backend/geonature/utils/config.py#L37

    error_module=ConfigError(CONFIG_FILE, e.messages) 

au lieu de lever une erreur on la stoque dans une variable error_config

dans app.py on importe error_config en meme temps que config et on pourrais ajouter ce test ici juste apres l'appel à Flask

https://github.com/PnX-SI/GeoNature/blob/7ee68c5555ecd4e3d1b252dd78254a492cd2d7eb/backend/geonature/app.py#L91-L99

    if error_config is not None:
        if os.environ["GEONATURE_SAFE_MODE_ON_CONFIG_FAILURE"]:
            print(config_error) # ou log de l'erreur)
            return
        else: 
            raise config_error

qui dans tout les cas va soit crasher l'appli, soit la lancer sans charger aucune route

Je me pose a question sur le comportement des commandes avec une config erronée idéalement il faudrait qu'elles crashent à cet endroit mais est ce qu'il est possible de savoir si on appelle create_app depuis une commande ?

camillemonchicourt commented 1 year ago

Je ne pense pas non plus qu'il faille aller trop loin dans un fonctionnement atypique et "maison" et qui apporte des risques en plus, pour pouvoir redémarrer automatiquement l'application quand on modifie sa configuration.