Log history v2.11 #1835

lpofredc · 2022-04-06T14:09:22Z

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 · 2022-04-06T14:19:25Z

Codecov Report

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

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

❗ 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%)`	⬇️

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%)`	⬇️
backend/geonature/core/gn_synthese/models.py	`90.41% <100.00%> (+1.12%)`	⬆️
backend/geonature/utils/config_schema.py	`91.73% <100.00%> (+<0.01%)`	⬆️
backend/geonature/core/gn_commons/repositories.py	`19.16% <0.00%> (-58.75%)`	⬇️
.../geonature/core/gn_permissions/backoffice/views.py	`28.00% <0.00%> (-57.44%)`	⬇️
backend/geonature/core/gn_permissions/routes.py	`42.10% <0.00%> (-51.65%)`	⬇️
backend/geonature/core/gn_profiles/routes.py	`33.85% <0.00%> (-45.51%)`	⬇️
...le_occhab/backend/gn_module_occhab/repositories.py	`13.98% <0.00%> (-43.53%)`	⬇️
backend/geonature/core/gn_commons/medias/routes.py	`35.38% <0.00%> (-37.35%)`	⬇️
backend/geonature/core/ref_geo/routes.py	`63.24% <0.00%> (-29.42%)`	⬇️
... 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.

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

DonovanMaillard · 2022-04-08T10:18:58Z

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 · 2022-04-08T10:18:58Z

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;

bouttier

Review partiel, il faut que je prenne un peu plus de temps pour me pencher sur l’utilisation de GenericQuery.

bouttier · 2022-04-06T18:57:50Z

backend/geonature/core/gn_synthese/routes.py

+
+if config["SYNTHESE"]["LOG_API"]:
+
+    @routes.route("/log", methods=["get"])


Plutôt que d’indenter l’entièreté de la fonction, tu peux la déclarer sans le décorateur route, et après celle-ci, mettre :

if config["SYNTHESE"]["LOG_API"]: routes.add_url_rule("/log", view_func=log_delete_history, methods=["GET"])

la fonction routes.add_url_rule n'est utilisé nulle part ailleurs dans GeoNature. Est-ce vraiment pertinent d'ajouter une nouvelle pratique? J'aurai tendance à rester sur l'usage du décorateur...

bouttier · 2022-04-06T18:58:53Z

backend/geonature/core/gn_synthese/routes.py

+    @routes.route("/log", methods=["get"])
+    @permissions.check_cruved_scope("R", True)
+    @json_resp
+    def log_delete_history(info_role) -> dict:


Nom induisant en erreur, on a l’impression que cette route sert à supprimer des entrées dans l’historique.
Suggestion : list_synthese_log_entries ?

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.

cf. lpoaura/GN2PG#4

La synchro se base sur l'id_synthese propre à chaque source. Mais il parait pertinent que le fournisseur exclue d'office les données sans UUID des données de synthèse transmises.

bouttier · 2022-04-06T18:59:40Z

backend/geonature/migrations/versions/9e9218653d6c_add_synthese_log_history.py

+    op.create_table(
+        "t_log_synthese",
+        sa.Column("id_synthese", sa.Integer, primary_key=True),
+        sa.Column("unique_id_sinp", UUID(as_uuid=True), nullable=False),


Le nullable=False me semble en trop.

bouttier · 2022-04-06T19:01:14Z

backend/geonature/core/gn_synthese/routes.py

+if config["SYNTHESE"]["LOG_API"]:
+
+    @routes.route("/log", methods=["get"])
+    @permissions.check_cruved_scope("R", True)


Ne devrions nous pas avoir un R sur le module SYNTHESE ?

DonovanMaillard · 2022-04-08T21:41:55Z

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 · 2022-04-08T21:51:46Z

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 · 2022-04-11T07:41:09Z

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")

bouttier · 2022-09-16T17:57:51Z

config/default_config.toml.example

+    # Ajoute une API requêtable de log listant sommairement les
+    # données mises à jour (id_synthese, uuid, dernière action, date de dernière action)
+    # Utilisée par GN2PG (https://github.com/lpoaura/GN2PG)
+    LOG_API = true


En fait il apparaît superflu de rajouter un paramètre de configuration pour activer / désactiver cette route de l’API.

camillemonchicourt · 2022-11-09T00:19:04Z

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

bouttier · 2022-12-24T16:07:40Z

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 · 2023-01-03T10:54:36Z

Quelques premières modif:

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

mvergez · 2023-01-26T16:47:53Z

Mise à jour du travail restant que nous allons effectuer :

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
Fix des conflits et MaJ en v2.11

DonovanMaillard · 2023-01-30T09:49:46Z

backend/geonature/core/gn_synthese/routes.py

 from utils_flask_sqla.response import to_csv_resp, to_json_resp, json_resp
 from utils_flask_sqla_geo.generic import GenericTableGeo

 from geonature.utils import filemanager
+from geonature.utils.config import config


toujours utile cet import ?

Improvingfeat: log history synthese module Improving PR [https://github.com/PnX-SI/GeoNature/pull/1835](https://github.com/PnX-SI/GeoNature/pull/1835) According to this comment : 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 Reviewed_by:andriac [Refs_PR]: PnX-SI#1835

andriacap · 2023-01-31T09:36:17Z

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 · 2023-01-31T16:51:58Z

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 · 2023-01-31T18:39:47Z

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

Improving PR PnX-SI#1835 According to this comment : 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 Reviewed_by:andriac [Refs_PR]: PnX-SI#1835

Improving PR #1835 According to this comment : 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 Reviewed_by:andriac [Refs_PR]: #1835

camillemonchicourt · 2023-02-13T14:03:01Z

PR regroupée sur le dépôt GeoNature - #2337

Improving PR #1835 According to this comment : 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 Reviewed_by:andriac [Refs_PR]: #1835

bouttier · 2023-03-10T17:12:12Z

PR reprise et intégré dans #2337

hypsug0 added 2 commits April 6, 2022 15:53

Add log history on synthese for CREATE UPDATE DELETE actions for gn2pg

536f368

fix: remove unused property

a8ddc28

lpofredc mentioned this pull request Apr 6, 2022

Log history #1456

Closed

hypsug0 added 2 commits April 6, 2022 16:25

fix: reformat code

96c4572

Merge remote-tracking branch 'upstream/develop' into log_history-v2.10

b6f6db5

bouttier reviewed Apr 8, 2022

View reviewed changes

hypsug0 added 4 commits April 11, 2022 10:07

fix: fix PR returns

74cd421

Merge remote-tracking branch 'upstream/develop' into log_history-v2.10

c0ec3f7

fix scope

fca748b

fix: apply black linting using black command from lint.sh

630ca3a

bouttier reviewed Sep 16, 2022

View reviewed changes

bouttier force-pushed the develop branch from 881c27a to f6ad309 Compare October 17, 2022 17:39

ophdlv mentioned this pull request Nov 2, 2022

Feat/parallelisation lpoaura/GN2PG#30

Merged

5 tasks

bouttier deleted the branch PnX-SI:develop November 8, 2022 16:09

bouttier closed this Nov 8, 2022

camillemonchicourt reopened this Nov 9, 2022

camillemonchicourt added this to the 2.12 milestone Dec 21, 2022

Merge tag '2.11.0' into log_history-v2.10

4227725

lpofredc changed the title ~~Log history v2.10~~ Log history v2.11 Jan 3, 2023

fix import

3699065

DonovanMaillard reviewed Jan 30, 2023

View reviewed changes

andriacap mentioned this pull request Jan 30, 2023

feat: log history synthese module andriacap/GeoNature#1

Closed

2 tasks

andriacap mentioned this pull request Jan 31, 2023

feat: log history synthese module #2311

Closed

2 tasks

andriacap mentioned this pull request Feb 2, 2023

improvement suggestion feat: log history synthese module lpofredc/GeoNature#38

Open

VincentCauchois assigned bouttier Feb 7, 2023

camillemonchicourt mentioned this pull request Feb 13, 2023

log history #2337

Merged

bouttier closed this Mar 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log history v2.11 #1835

Log history v2.11 #1835

lpofredc commented Apr 6, 2022

codecov bot commented Apr 6, 2022 •

edited

Loading

DonovanMaillard commented Apr 8, 2022 •

edited

Loading

DonovanMaillard commented Apr 8, 2022

bouttier left a comment

bouttier Apr 6, 2022

lpofredc Apr 11, 2022

bouttier Apr 6, 2022

lpofredc Apr 11, 2022

bouttier Apr 6, 2022

bouttier Apr 6, 2022

DonovanMaillard commented Apr 8, 2022

bouttier commented Apr 8, 2022

lpofredc commented Apr 11, 2022

bouttier Sep 16, 2022

camillemonchicourt commented Nov 9, 2022

bouttier commented Dec 24, 2022

lpofredc commented Jan 3, 2023

mvergez commented Jan 26, 2023 •

edited

Loading

DonovanMaillard Jan 30, 2023

andriacap commented Jan 31, 2023 •

edited

Loading

bouttier commented Jan 31, 2023

andriacap commented Jan 31, 2023

camillemonchicourt commented Feb 13, 2023

bouttier commented Mar 10, 2023


		if config["SYNTHESE"]["LOG_API"]:

		@routes.route("/log", methods=["get"])

Log history v2.11 #1835

Log history v2.11 #1835

Conversation

lpofredc commented Apr 6, 2022

codecov bot commented Apr 6, 2022 • edited Loading

Codecov Report

DonovanMaillard commented Apr 8, 2022 • edited Loading

DonovanMaillard commented Apr 8, 2022

bouttier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DonovanMaillard commented Apr 8, 2022

bouttier commented Apr 8, 2022

lpofredc commented Apr 11, 2022

Choose a reason for hiding this comment

camillemonchicourt commented Nov 9, 2022

bouttier commented Dec 24, 2022

lpofredc commented Jan 3, 2023

mvergez commented Jan 26, 2023 • edited Loading

Choose a reason for hiding this comment

andriacap commented Jan 31, 2023 • edited Loading

bouttier commented Jan 31, 2023

andriacap commented Jan 31, 2023

camillemonchicourt commented Feb 13, 2023

bouttier commented Mar 10, 2023

codecov bot commented Apr 6, 2022 •

edited

Loading

DonovanMaillard commented Apr 8, 2022 •

edited

Loading

mvergez commented Jan 26, 2023 •

edited

Loading

andriacap commented Jan 31, 2023 •

edited

Loading