Log history v2.10

[lpofredc]

PR intégrant une historisation des données de la synthèse (INSERT, UPDATE & DELETE):

Remplace la PR #1456

• Les DELETE sont loggés dans la table gn_synthese.t_log_synthese par triggers
• Les UPDATE & INSERT ne sont pas loggés par triggers car déjà présents dans gn_synthese.synthese par les champs meta_{create,update}_date.
• Le tout est regroupé dans une vue gn_synthese.v_log_synthese.

L'API, désactivable par l'option ['SYNTHESE']['LOG_API'] = False, est basée sur la classe GenericQuery donc avec des possibilités de requêtage avancées (action supérieure à une date spécifique par exemple).

Cette API est nécessaire pour la mise à jour incrémentale de Gn2Pg.

Lien avec #789

[codecov[bot]]

Codecov Report

Base: 67.81% // Head: 54.61% // Decreases project coverage by -13.19% :warning:

Coverage data is based on head (630ca3a) compared to base (25fb18c). Patch coverage: 59.25% of modified lines in pull request are covered.

:exclamation: Current head 630ca3a differs from pull request most recent head 3699065. Consider uploading reports for the commit 3699065 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #1835       +/-   ##
============================================
- Coverage    67.81%   54.61%   -13.20%     
============================================
  Files           83       76        -7     
  Lines         7369     7318       -51     
============================================
- Hits          4997     3997     -1000     
- Misses        2372     3321      +949     

Flag	Coverage Δ
pytest	54.61% <59.25%> (-13.20%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
backend/geonature/core/gn_synthese/routes.py	76.77% <38.88%> (-2.20%)	:arrow_down:
backend/geonature/core/gn_synthese/models.py	90.41% <100.00%> (+1.12%)	:arrow_up:
backend/geonature/utils/config_schema.py	91.73% <100.00%> (+<0.01%)	:arrow_up:
backend/geonature/core/gn_commons/repositories.py	19.16% <0.00%> (-58.75%)	:arrow_down:
.../geonature/core/gn_permissions/backoffice/views.py	28.00% <0.00%> (-57.44%)	:arrow_down:
backend/geonature/core/gn_permissions/routes.py	42.10% <0.00%> (-51.65%)	:arrow_down:
backend/geonature/core/gn_profiles/routes.py	33.85% <0.00%> (-45.51%)	:arrow_down:
...le_occhab/backend/gn_module_occhab/repositories.py	13.98% <0.00%> (-43.53%)	:arrow_down:
backend/geonature/core/gn_commons/medias/routes.py	35.38% <0.00%> (-37.35%)	:arrow_down:
backend/geonature/core/ref_geo/routes.py	63.24% <0.00%> (-29.42%)	:arrow_down:
... and 55 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[DonovanMaillard]

Merci Frédéric,

Une question/remarque au passage : dans le downgrade du fichier de migration alembic, on ne supprime pas la table t_log_synthese qui est créée :

DROP TABLE IF EXISTS gn_synthese.t_log_synthese;

Une seconde : dans le config_schema.py, c'est la variable missing=true qui est restée sur le nouveau paramètre. Ne faut-il pas passer à load_default=true ?

[DonovanMaillard]

Merci Frédéric,

Une question/remarque au passage : dans le downgrade du fichier de migration alembic, on ne supprime pas la table t_log_synthese qui est créée :

DROP TABLE IF EXISTS gn_synthese.t_log_synthese;

[DonovanMaillard]

Suggestion si besoin de renommer "list_synthese_deletions"

le NOT NULL sur l'uuid sinp semble quand meme intéressant pour pas faire transiter des données sans identifiants (et il est sans doute obligatoire pour gérer l'incrémental d'ailleurs)

[bouttier]

Suggestion si besoin de renommer "list_synthese_deletions"

La route ne renvoi pas que les logs de suppression mais également de création et modification.

le NOT NULL sur l'uuid sinp semble quand meme intéressant pour pas faire transiter des données sans identifiants (et il est sans doute obligatoire pour gérer l'incrémental d'ailleurs)

Le module d’export permet d’exporter les données sans UUID (forcer l’UUID peut amener certains détenteur de données à générer des UUID pour des données dont ils n’ont pas la connaissance de l’UUID et dont ils ne sont pas les producteurs, amenant potentiellement à générer plusieurs UUID pour la même donnée) → il faut pouvoir gérer les données sans UUID.

La synchronisation doit donc se fonder sur l’id_synthese, qui est la clé primaire dont l’unicité est garantie pour une instance donnée (et qui sera stocké dans le champs entity_source_pk_value chez la destination). Je ne sais plus où, mais @hypsug0 m’a confirmé que c’est bien le comportement adopté par GN2PG.

[lpofredc]

Merci Frédéric,

Une question/remarque au passage : dans le downgrade du fichier de migration alembic, on ne supprime pas la table t_log_synthese qui est créée :

DROP TABLE IF EXISTS gn_synthese.t_log_synthese;

Elle est supprimée par la commande précédente:

op.drop_table("t_log_synthese", schema="gn_synthese")

[camillemonchicourt]

PR fermée par erreur suite à la suppression accidentelle de la branche DEVELOP

[bouttier]

Après discussions, afin de terminer cette PR :

• Supprimer le paramètre LOG_API (superflu)
• Supprimer l’usage de GenericQuery : la manière fléché par SQLAlchemy pour cela serait d’utiliser query_class ; on peut envisager de faire des fonctions de filtrage générique de cette manière mais il faut que cela soit bien réfléchit et bien couvert par les tests.
• Supprimer la vue v_log_synthese (plus difficile à maintenir) au profit d’une requête d’union directement dans la route
• L’UUID est-il vraiment nécessaire ? Si non, ne garder que l’id_synthese
• Renommer le modèle TLogSynthese en SyntheseLogEntry
• Vérifier la cohérence entre le modèle et le SQL de création de la table (par exemple, supprimer le NOT NULL sur le champs d’UUID dans le SQL)
• Écrire des tests unitaires

[lpofredc]

Quelques premières modif:

• Modèle TLogSynthese renommé en SyntheseLogEntry
• Paramètre LOG_API supprimé
• Fix des conflits et MaJ en v2.11

[mvergez]

Mise à jour du travail restant que nous allons effectuer :

• [x] Supprimer le paramètre LOG_API (superflu)
• [ ] Supprimer l’usage de GenericQuery : la manière fléché par SQLAlchemy pour cela serait d’utiliser query_class ; on peut envisager de faire des fonctions de filtrage générique de cette manière mais il faut que cela soit bien réfléchit et bien couvert par les tests.
• [x] Supprimer la vue v_log_synthese (plus difficile à maintenir) au profit d’une requête d’union directement dans la route
• [ ] L’UUID est-il vraiment nécessaire ? Si non, ne garder que l’id_synthese
• [x] Renommer le modèle TLogSynthese en SyntheseLogEntry
• [ ] Vérifier la cohérence entre le modèle et le SQL de création de la table (par exemple, supprimer le NOT NULL sur le champs d’UUID dans le SQL)
• [ ] Écrire des tests unitaires
• [x] Fix des conflits et MaJ en v2.11

[andriacap]

Mise à jour du travail restant que nous allons effectuer :

* [x]  Supprimer le paramètre LOG_API (superflu)

* [ ]  Supprimer l’usage de GenericQuery : la manière fléché par SQLAlchemy pour cela serait d’utiliser query_class ; on peut envisager de faire des fonctions de filtrage générique de cette manière mais il faut que cela soit bien réfléchit et bien couvert par les tests.

* [ ]  Supprimer la vue v_log_synthese (plus difficile à maintenir) au profit d’une requête d’union directement dans la route

* [ ]  L’UUID est-il vraiment nécessaire ? Si non, ne garder que l’id_synthese

* [x]  Renommer le modèle TLogSynthese en SyntheseLogEntry

* [ ]  Vérifier la cohérence entre le modèle et le SQL de création de la table (par exemple, supprimer le NOT NULL sur le champs d’UUID dans le SQL)

* [ ]  Écrire des tests unitaires

* [x]  Fix des conflits et MaJ en v2.11

Bonjour tout le monde,

Hier j'ai fork le projet GeoNature et j'ai récupéré la branche develop depuis le commit 7092f1c datant du 27 Janvier. A partir de ce commit , j'ai récupéré les changements apportés de cette PR. J'ai travaillé sur les deux points suivants voir la PR sur mon dépot :

• [ ] Supprimer l’usage de GenericQuery : la manière fléché par SQLAlchemy pour cela serait d’utiliser query_class ; on peut envisager de faire des fonctions de filtrage générique de cette manière mais il faut que cela soit bien réfléchit et bien couvert par les tests.
• [ ] Supprimer la vue v_log_synthese (plus difficile à maintenir) au profit d’une requête d’union directement dans la route

Pour résumé ce qui a été fait :

• J'ai enlever la création de la vue matérialisée dans le fichier de migration
• J'ai adapté la route @routes.route("/log", methods=["get"]) en changeant le GenericQuery par l'utilisation de la SyntheseQuery dans laquelle j'ai ajouté des méthodes pour pouvoir filter et sort . J'ai également écrit le SELECT .. UNION.. qui était fait à la base dans la vue matérialisé (voir : https://github.com/andriacap/GeoNature/blob/146ce1aec3ee92583f668fb34fd595bd45ba06c3/backend/geonature/core/gn_synthese/routes.py#L1195-L1239 )
• J'ai rajouté un fichier dans le dossier utils du gn_synthese pour implémenter les méthodes filter, sort, paginate utilisé par la SyntheseQuery
  • voir backend/geonature/core/gn_synthese/utils/routes.py (fichier ajouté)
  • voir https://github.com/andriacap/GeoNature/blob/146ce1aec3ee92583f668fb34fd595bd45ba06c3/backend/geonature/core/gn_synthese/models.py#L133-L163 (ajout de méthode à la classe SyntheseQuery déjà existante)

L'extension de la classe BaseQuery en ajoutant les méthode de filtre, sort, paginate est basé sur le travail réalisé avec @mvergez dans le cadre du travail sur le module monitoring avec le MNHN.

Concernant le travail sur les autres points , notamment la partie test je voulais savoir si vous avez un exemple de fixture que pourrait retourner pour valider la réponse de la route /synthese/log .

Merci d'avance pour la lecture de ce message et bonne journée

[bouttier]

Hello @andriacap, peux-tu ouvrir une PR directement sur GeoNature pour faciliter la relecture et les retours ? Merci ! Je ne crois pas qu’il y ait de fixture pour la mise-à-jour ou la suppression de données de la synthèse.

[andriacap]

Hello @andriacap, peux-tu ouvrir une PR directement sur GeoNature pour faciliter la relecture et les retours ? Merci ! Je ne crois pas qu’il y ait de fixture pour la mise-à-jour ou la suppression de données de la synthèse.

Salt Elie , c'est bon pour la PR . Bonne soirée

[camillemonchicourt]

PR regroupée sur le dépôt GeoNature - https://github.com/PnX-SI/GeoNature/pull/2337

[bouttier]

PR reprise et intégré dans #2337