leximpact / leximpact-server

Simuler l'impact des réformes socio-fiscales en moins d'une minute
https://leximpact.an.fr
GNU Affero General Public License v3.0
4 stars 0 forks source link

Var env issue11 #14

Closed magemax closed 4 years ago

magemax commented 4 years ago

Création de 3 super variables d'environnements pour répondre à l'issue 11 :

Questions résiduelles :

Reste à faire :

@sandcha pas besoin à ce stade de review le code, les "questions résiduelles" portent sur la definition of done. Tu auras tout au plus besoin de jeter un coup d'oeil pour comprendre toutes les questions résiduelles.

sandcha commented 4 years ago

Tout d'abord, challenge accepted pour les cas types avec un "emoji" des années 90 ! 😸

                ________________
                |    Cas types   |_____    __
                |    forever!    |     |__|  |_________
                |________________|     |::|  |        /
   /\**/\       |                \.____|::|__|      <
  ( o_o  )_     |                      \::/  \._______\
   (u--u   \_)  |
    (||___   )==\
  ,dP"/b/=( /P"/b\
  |8 || 8\=== || 8
  `b,  ,P  `b,  ,P
    """`     """`
sandcha commented 4 years ago

@magemax Merci pour ces améliorations ! Voici quelques réponses (toujours dispo si cela manquait de précision) :

Mes méthodes de "fail gracefully et fast" sont noramment présente dans l'implémentation du PLF_PATH. Pour l'instant j'utilise la technique dite du print & raise, qui est pas très classe. Comment logger de manière plus classe ?

Une manière de logger est d'utiliser un logger qui a l'avantage de pouvoir être paramétré. Par exemple, si l'on souhaite définir certaines traces dedebug qui ne s'afficheraient que lorsqu'on le demande (pas tout le temps parce que c'est trop verbeux pour le quotidien). En Python, il y a la librairie logging. Voici un exemple d'utilisation sur un logiciel que tu connais bien. Le principe est d'attraper l'exception, s'il s'agit d'une erreur (pas d'un élément secondaire attendu) conserver sa trace mais d'assurer que le message en fin de prompt que verra l'usager est reformulé pour donner une piste de solution contextualisée à la situation. Enfin, on renvoie le tout sur le logger avec le niveau ad hoc (log.info, log.warning, log.error...).

les noms de variable sont pour l'instant moches : j'ai pas changé DATA_PATH et j'ai rajouté PLF_PATH. Des suggestions ? Les autres noms sont en anglais par cohérence.

Je dirais que c'est principalement EMPIRIC_VALUE qui n'est pas explicite. A priori, si c'est une variable, value n'apporte pas beaucoup d'information. Par ailleurs, on peut se demander value de quoi ? De quelle unité si on veut la modifier ? Une proposition : RECETTES_ETAT_EURO

PLF_PATH et DATA_PATH me semblent bien s'ils désignent bien un chemin. Si le chemin est contraint et doit être relatif à un point de l'application, je dirais PLF_LOCAL_PATH. Si l'on attend un nom de fichier en valeur (pas un répertoire), je conseillerais FILEPATH. Si le répertoire est pré-défini et qu'on souhaite juste le nom du fichier, je dirais FILENAME.

Il me semble que les cas types sont configurables de la même manière que le PLF mais ça peut se faire plus tard si tu le préfères.

magemax commented 4 years ago

OK donc en résumé cette PR contient à ce stade :

@sandcha on en parle quand tu veux

sandcha commented 4 years ago

Au retour sur cette branche avec un .env local mal configuré, j'ai obtenu une pile d'erreur se terminant par :

virtualenvs/lis22/lib/python3.7/site-packages/openfisca_core/periods.py:840: in raise_error
    raise ValueError(message)
E   ValueError: Expected a period (eg. '2017', '2017-01', '2017-01-01', ...); got: 'None'.
E   Learn more about legal period formats in OpenFisca:
E   <https://openfisca.org/doc/coding-the-legislation/35_periods.html#periods-in-simulations>.

Afin d'adapter ce message d'erreur, j'ai introduit une ConfigurationException dans les derniers commits.