PnX-SI / gn_module_import

Module GeoNature d'import de données
7 stars 11 forks source link

Refactorisation du module #257

Closed camillemonchicourt closed 2 years ago

camillemonchicourt commented 2 years ago

Une refactorisation complète du module a été initiée par @bouttier en 2021. Elle est initiée dans la branche dédiée : https://github.com/PnX-SI/gn_module_import/compare/develop...refactor

Évolutions prévues et initiées

Le travail doit continuer pour faire :


Détail des taches prévues, à spécifier :

1. Interdire l'import de données dont la date d'observation est antérieure à la date du jour

A l'heure actuelle des données dont la date d'observation est ultérieure à la fate du jour peuvent être iportées sans problème dans GeoNature. Actuellement il n'existe que 2 contrôles sur les dates au niveau du module d'import :

Ajouter les contrôles à l'import vérifiant que date_min et date_max sont inférieures ou égales à date du jour.

Ajouter Un warning à l'import vérifiant que date_min et date_max sont inférieures ou égales à 1900 > l'info est remontée dans le rapport d'erreur mais les données sont considérées comme valides.


2. Choix du délimiteur dans les paramètres de chargement du fichier

Même principe que pour l’encodage : après upload du fichier, le délimiteur est auto-détecté. À l’étape suivante, le délimiteur est pré-sélectionné sur la valeur auto-détecté mais l’utilisateur peut modifier ce choix.


3. Rendre scalable le check des UUID déjà dans la synthèse


4. Rendre paramétrable le check de l'unicité de U_id_sinp sur l'ensemble de la synthèse ou par jeu de donné


5. Vérification du référentiel cd_nom

Vérifier que les cd_nom sont dans le référentiel TaxRef


6. Vérification du référentiel cd_hab

Vérifier que les cd_nom sont dans le référentiel HabRef


7. Entity source pk : vérifier les doublons (DUPLICATE_ENTITY_SOURCE_PK)


8. ID Digitizer: récupérer id à partir MTD INPN

L’id_digitizer est une FK vers id_role. Dans le cas où l’utilisateur n’existe pas dans la table utilisateurs.t_roles (typiquement l’utilisateur ne s’est jamais connecté à DEPOBIO), il faut importer l’utilisateur depuis MTD.

Si l’utilisateur n’a pas pu être récupérer depuis MTD => erreur sur la ligne de l’import : digitizer inconnu

CM : Si un utilisateur ne s'est jamais connecté à DEPOBIO, je ne vois pas comment il pourrait être digitizer ? JP : Ce cas d'usage me est possible sur nos instances. ex. L'utilisateur a créé des MTD sur l'INPN mais ne s'est pas encore connecte a GeoNature. Entre temps l'administrateur est connecté et toutes les MTD sont récupérées de l'INPN . Nous avons mis en place un CRON automatique de récupération de toutes les MTD de la plateforme.


9. Génération des altitudes

Voir comment c’est fait actuellement.

À priori :


10. Reporter le nom original des colonnes dans les erreurs

Dans le rapport d’erreur, on veut le nom de la colonne du CSV pour que l’utilisateur soit en mesure de retrouver son erreur.

Dans l’objet Import, il y a un tableau d’association colonne PostgreSQL / colonne CSV, utile pour les conversions.


11. Vérifier URL de digital_proof


12. Vérifier les lignes dupliquées (DUPLICATE_ROWS)


13. Ajout du support du format geojson

En v1: le geojson était convertie en CSV. Sans doute mieux d’importer directement le geojson dans la table. À voir s’il est possible de présélectionner le champs de géométrie à l’étape de mapping des champs.


14. Limitation de la taille du fichier


15. Supprimer goodtables

Vérifier la validité du fichier CSV lors de l’import en base via panda.


16. Tests unitaires > Permissions sur JDD

Vérifier qu’on import sur un JDD sur lequel on a les droits. Peut-être déjà fait, mais :


17. Tests Code maille

Test unitaire sur la validité de code maille.


18. Tests Code commune

Test unitaire sur la validité de code commune.


19. Tests Code département

Test unitaire sur la validité de code département.


20. Gérer différent types de code maille


21. Gestion de TypeInfoGeo

Le champs TypeInfoGeo sert à indiquer, lorsque plusieurs informations géographique sont attachées à une observation, quelle est l’information géographique de référence (on doit toujours avoir une information géographique avec TypeInfoGeo=géolocalisation, les autres informations géographique ont donc TypeInfoGeo=rattachement).

GeoNature ne gère qu’une seule information géographique par observation. Cette information géographique est donc nécessairement de type géolocalisation. Dans ce cadre, ce champs n’a pas lieu d’être dans GeoNature. Il peut donc être retiré du module d’import.

Cependant, une autre information est utile : DEE_FLOUTAGE, permettant d’indiquer s’il s’agit d’une information géographique précise ou dégradé. Il faudrait rajouter la possibilité d’indiquer cette information dans le module d’import. Pas de check associé à ce champs (toutes les combinaisons sont possible, geom de type point avec FLOUTAGE=OUI (typiquement centroïde), geom de type commune avec FLOUTAGE=NON - malheuresement la commune peut être l’information la plus précise que l’on a).


22. [Backend] Suppression table après import


23. [Backend] Suppression du schéma archives


24. [Backend] Gestion des erreurs à chaque étapes


25 [Backend] Création d’un n° unique de requête, affichage dans les erreurs, et ajout d’un tag dans sentry


26. [Backend] Migration alembic pour 1.1.x => 2.0.0


27. [Backend] Import asynchrone


28. [Frontend] Gestion des mappings

API OK, reste de la gestion en front, surtout côté valeurs


29. [Frontend] Gestion des steps : suppression du field « step », détermination du step à partir des données de l’import


30. [Frontend] Liste d’import : nettoyage champs à partir du modèle d'import (import, nom, encodage)


31. [Frontend] Gestion reprise de l’import

https://github.com/PnX-SI/gn_module_import/blob/refactor/frontend/app/components/import_process/import-process.resolver.ts


32. [Frontend] Gestion de l’asynchrone


33. Documentation

Mise à niveau de la documentation technique GeoNature


34. Pouvoir spécifier dans la configuration la liste des checks à exécuter

Plutôt que de rajouter une option dans la conf pour chaque check que l’on souhaite rendre optionnel, définir un paramètre qui liste les checks à appliquer. Il devient alors facile de pouvoir désactiver des checks en les retirant de cette liste.

Idéalement, cette liste doit permettre de rajouter des checks non prévu (test spécifique à mon instance).

Deux idées pour cela :

Pour chaque check, pouvoir indiquer si le check est bloquant ou non (error vs warning)


35. Création table de données d'import dédiée


36. [Frontend] Composant générique d'affichage des erreurs

Handler sur les 500 qui appelle le toaster par exemple

DonovanMaillard commented 2 years ago

Merci pour cette info. Quelques premiers retours en attendant d'avoir des détails pour les différents points qui le nécessiteraient :

A voir si des échanges peuvent être faits points par points quand ça dépasse le simple volet technique

camillemonchicourt commented 2 years ago

Suite aux échanges :

DonovanMaillard commented 2 years ago

28 [Frontend] Gestion des mappings / Inverser le sens de mapping au niveau des nomenclatures. Chaque sens a des avantages et inconvénients

A ce stade le changement a été développé et implémenté dans la branche de refonte. Le changement sera laissé pour la release, avec ses avantages et ses inconvénients. D'une manière générale, l'amélioration de l'ergonomie viendra à terme du fait que le module fasse intelligemment des propositions de mapping pertinentes quand on crée un nouveau mapping , et non pas du sens de l'affichage...

bouttier commented 2 years ago

Nouveau modèle de données pour les mappings. mappings Les mappings sont stockés entièrement dans les champs t_imports.fieldmapping et t_imports.contentmapping. Le modèle t_mappings correspond exclusivement aux modèles d’import. Les tables t_contentmappings et t_fieldmappings permettent de stocker les associations des modèles dans le champs values. Les champs t_fieldmappings.values / t_imports.fieldmapping sont de type HSTORE, il s’agit d’un tableau d’association de la forme suivante :

{
    champs de destination ⇒ champs source
}

Les champs t_contentmappings.values / t_imports.contentmapping sont de type JSON, le JSON à la forme suivante :

{
    mnemonique du type de nomenclature ⇒ {
        valeur source ⇒ cd_nomenclature de la nomenclature de destination
    }
}

Les modèles sont modifiables depuis Flask-Admin. Une validation vérifie le bon formatage du JSON, l’existence des mnémonique / des cd_nomenclature, ainsi que des champs de destination pour les correspondances de champs.

DonovanMaillard commented 2 years ago

Top, merci pour cette amélioration sur les mappings, dissocier les modèles et les mappings réellement utilisés était essentiel !

Pour la t_import, tu as laissé pour le moment le champ import_table.

Veux tu qu'on ré-échange à l'occasion pour modifier le stockage des données sources ? je proposais quelque chose comme :

Ca peut faire une table un peu complexe à l'arrivée, mais ca évite de multiplier les tables dans ce schéma (un soucis dans certaines instances) et ça permet de travailler/récupérer/retélécharger les données invalides pour conserver cette fonctionnalité.

bouttier commented 2 years ago

Nouvelle proposition :

L’utilisateur téléverse son fichier, choisie l’encodage, le délimiteur, etc. À ce moment là, on lit uniquement la première ligne du fichier CSV de manière à lire les colonnes du fichier ; celles-ci sont stocké dans t_import.columns. Les données ne sont pas encore lue !

On passe aux correspondances de champs. Pour cela, on lit les colonnes source du CSV depuis t_import.columns et les colonnes de destination depuis dict_fields comme actuellement. Lorsque les correspondances de champs sont validés, les données du fichier CSV sont chargé dans la table t_import_data, en plaçant les données dans le jeu de colonnes « source ». Cette table contient les données de tous les imports, mais la FK t_import.id_import permet de retrouver les données de notre import uniquement.

On passe aux correspondance des valeurs. On utilise la table t_import_data pour retrouver les valeurs disponibles à l’import. Après validation est lancé l’étape de transformation qui va placé dans le jeu de colonne de destination les valeurs après transformation.

On peut ici effectuer en SQL toutes les modifications des données que l’on souhaite. On utilisera simplement des noms de colonnes qui correspondent aux colonnes de la synthèse plutôt qu’au colonne du fichier CSV (c’est à mon avis même plus simple).

Puis on passe à l’étape d’import des données dans la synthèse, qui est alors extrêmement simple.

DonovanMaillard commented 2 years ago

Hmmm un peu gymnastique pour retrouver là dedans quelque chose qui ressemble au fichier source (on perd les infos qu'on ne rattache à rien (mais peu importe on a choisi de les ignorer), on n'a plus notre nom de colonne donc on doit retourner voir la correspondance.... Je pense que dans la pratique, retourner sur la donnée source sera "complexe" ou moins intuitif mais faisble. En faisant ca, on perd l'a donnée des champs non mappés (donc si on veut modifier ou compléter le mapping, on a toujours le nom de la colonne mais on a pu la donnée ?.

A voir aussi comment se traite le champs json additionnal_data de la syntèhse (plusieurs valeurs de plusieurs champs en entrée). Pourquoi pas, je suis pas sur de tout voir très clair tout de suite mais je suis pas fermé à cette facon de faire non plus.

Petite question quand même. Si on stocke le contenu du fichier source dans un champs binaire. Il existe des fonctions (ou une possibilité de faire une fonction) qui permette de "remonter" une table du fichier source à partir du binaire ?

bouttier commented 2 years ago
DonovanMaillard commented 2 years ago

On perd les colonnes qu’on ne rattache à rien : en effet

Donc on aurait certains champs en json, certains réorganisés selon le mapping au format synthèse ... pourquoi pas

On n’a plus la structure source : est réellement important ?

Dans la base non, je pense que c'est surtout une question d'habitude quand on travaille sur les données, et encore une fois pour une manip qu'on fait quand meme qu'occasionnellement de remonter à la source... pas bloquant en effet, mais c'est un choix de se dire qu'on stocke uniquement un format transformé

pour le json additionnal_data ok donc le champs source et cible seraient sous le même format, et pas au format source (mais ni genant ni d'autres solutions avec la possibilité que tu proposes)

Le besoin de retrouver d'une manière ou d'une autre le fichier source est intéressant et souhaité dès la conception du module, pour pouvoir remonter au fournisseur de données ce qu'il nous a transmises ou non à un instant t par exemple, pour qu'il puisse nous transmettre un différentiel, des données à jour etc, ou pour qu'on puisse revenir nous-mêmes sur le mapping qu'on a fait, voir si on a des choses à corriger ou améliorer pour mieux traduire les données source qu'on a reçues vers le format SINP. Notamment quand il y a des changements de référentiels ou de nouveaux champs, qui dans certains cas peuvent être alimentés depuis la source et plus depuis les DEE qu'on en a tirées.

En fait pour résumer les besoins (à mon sens) liés à cette question :

Du coup entre ce que tu proposes ici ou un stockage binaire qui conserve ces possibilités (donc une table d'import dans la même logique qu'aujourd'hui, mais supprimée après l'import) sont toutes les deux possibles. A voir ce qui est le plus propre. Le choix entre les deux doit surtout être un choix technique lié aux performances à priori...

bouttier commented 2 years ago

DEE ?

Sinon l’ensemble des contraintes que tu as soulevés sont à mon sens compatible avec la solution proposée. Tout en étant plus performant et plus propre.

Pour préciser le processus :

Téléchargement des données invalides : à partir du CSV source, et non de la table t_imports_data qui ne contient pas l’entièreté des données du fichier CSV (du moins pas directement) (on utilise tous de même la table t_imports_data pour avoir les numéros des lignes erroné à renvoyer à l’utilisateur).

La synthèse est re-calculable : à partir du fichier csv source et des 2 mappings, on peut re-généré les données des colonnes « destination ». Est-ce donc utile de garder ces données à la fin du processus d’import ? Non pou les colonnes « source », oui pour les colonnes « destination » si les données ont été manuellement modifié, les garde-t-on ?

camillemonchicourt commented 2 years ago

Ok, merci pour ces échanges et précisions.

De ma compréhension, si on va vers cela, on n'a pas besoin de garder les données intermédiaires après l'import. Seulement les données sources brutes et la possibilité d'exporter les données invalides.

bouttier commented 2 years ago

On a 4 fois les données :

⇒ Il apparaît inutile de garder les colonnes « source » et « destination » à la fin du processus d’import.

Se pose aussi la question de l’utilité d’avoir 2 fois les colonnes de la synthèse. Pour les nomenclatures, c’est nécessaire car on les transformes après l’étape de correspondance des valeurs. En revanche, cela apparaît inutile pour les autres colonnes. Ceci dit, on pourra trouver dans les colonnes dites « source » des colonnes qui ne sont pas dans la synthèse, celles référencés dans dict_fields avec synthese_field=False : codecommune, codemaille, longitude, hour_min, … qui servent à alimenter les colonnes de la synthèse après transformation.

DonovanMaillard commented 2 years ago

DEE

Données élémentaires d'échanges selon les termes du SINP. En clair, les données de synthèse.

Ceci dit, on pourra trouver dans les colonnes dites « source » des colonnes qui ne sont pas dans la synthèse

Oui, on peut notamment avoir un champs qu'on ne mappe avec rien, qu'on manipule pour le splitter, et utiliser ces champs calculés pour l'import. Celà dit, on peut toujours avoir comme tu disais un json qui contient tous les champs non mappés derrière les champs de synthèse en double.

Si on a stocké les mappings, qu'on peut les télécharger, les réimporter, qu'on peut récupérer le fichier source. On peut à mon sens ne pas garder les lignes des imports terminés dans la table t_import_data ... On perdra certes les infos calculées, mais ça reste un cas d'usage qui , même si on souhaite le conserver, ne doit pas plomber tout le fonctionnement normal du module.

Du coup :

Pour l'export des données invalides, s'il faut reconstituer une table à partir du binaire et aller piocher dedans les lignes en erreur, on peut imaginer d'avoir comme pour le module export un envoi de mail quand le fichier est dispo.

De cette manière :

Pour la pertinence d'avoir tous les champs de la synthèse ou non en double... : "En revanche, cela apparaît inutile pour les autres colonnes."

camillemonchicourt commented 2 years ago

La refonte du module est bien avancée et fonctionnelle. Voici les évolutions réalisées :

Évolutions techniques

Évolutions fonctionnelles

Permissions

Processus

camillemonchicourt commented 2 years ago

Refonte réalisée dans la version 2.0.0 avec donc une reprise de fond de tout le code, l'ajout de tests automatisés (couvrant 91% du code backend).

La migration Alembic de la version 1.x à 2.x permet d'appliquer automatiquement les évolutions importantes de la BDD, dont la suppression du schéma d'archive (après avoir récupérer les données sources pour les importer dans le champs gn_imports.t_imports.source_file) ainsi que les tables intermédiaires des imports réalisés en v1.


Reste quelques régressions à traiter par ailleurs :

Et l'ajout de la possibilité de télécharger en CSV les données sources d'un import directement depuis l'interface (possible depuis DBeaver ou autre actuellement).


Nouveau MCD du module :

geonature2db_demo - gn_imports