Add aggregator parser #3196

LePetitTim · 2022-07-28T15:18:47Z

No description provided.

cypress · 2022-07-28T15:31:01Z

Test summary

22 • 0 • 0 • 0

Run details


Project	Geotrek-admin
Status	Passed
Commit	`fa4ed9e` ℹ️
Started	Sep 19, 2022 3:45 PM
Ended	Sep 19, 2022 3:49 PM
Duration	04:14 💡
OS	Linux Ubuntu - 20.04
Browser	Electron 94

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

codecov · 2022-07-28T17:30:05Z

Codecov Report

Base: 97.94% // Head: 98.10% // Increases project coverage by +0.16% 🎉

Coverage data is based on head (b72044e) compared to base (34c4143).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3196      +/-   ##
==========================================
+ Coverage   97.94%   98.10%   +0.16%     
==========================================
  Files         273      275       +2     
  Lines       19262    19767     +505     
==========================================
+ Hits        18866    19393     +527     
+ Misses        396      374      -22

Impacted Files	Coverage Δ
geotrek/api/v2/serializers.py	`99.88% <100.00%> (ø)`
geotrek/common/management/commands/import.py	`100.00% <100.00%> (ø)`
geotrek/common/parsers.py	`91.54% <100.00%> (+5.97%)`	⬆️
geotrek/core/filters.py	`100.00% <100.00%> (ø)`
geotrek/core/models.py	`96.69% <100.00%> (+0.05%)`	⬆️
geotrek/infrastructure/filters.py	`100.00% <100.00%> (ø)`
geotrek/infrastructure/models.py	`100.00% <100.00%> (ø)`
geotrek/infrastructure/parsers.py	`100.00% <100.00%> (ø)`
geotrek/outdoor/filters.py	`100.00% <100.00%> (ø)`
geotrek/outdoor/models.py	`99.67% <100.00%> (+0.01%)`	⬆️
... and 24 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.

numahell

Je fais un première review, il manque le changelog. Je regarde le reste plus tard.

numahell · 2022-08-02T09:17:15Z

geotrek/common/management/commands/import.py

@@ -12,7 +12,7 @@ class Command(BaseCommand):

    def add_arguments(self, parser):
        parser.add_argument('parser', help='Parser class name in var/conf/parsers.py (or dotted syntax in python path)')
-        parser.add_argument('shapefile', nargs="?")
+        parser.add_argument('filename', nargs="?")


This should be documented (in install/imports.rst maybe)

numahell · 2022-08-02T15:56:24Z

geotrek/common/management/commands/import.py

@@ -12,7 +12,7 @@ class Command(BaseCommand):

    def add_arguments(self, parser):
        parser.add_argument('parser', help='Parser class name in var/conf/parsers.py (or dotted syntax in python path)')
-        parser.add_argument('shapefile', nargs="?")
+        parser.add_argument('filename', nargs="?", help='Optional file which will be use to feed database')


Suggested change

parser.add_argument('filename', nargs="?", help='Optional file which will be use to feed database')

parser.add_argument('filename', nargs="?", help='Optional file used to feed database')

plus court, c'est mieux

numahell

Je continue de lire et essayer de tout comprendre, voilà déjà des retours

numahell · 2022-08-03T14:07:54Z

geotrek/common/parsers.py

+
+class GeotrekParser(AttachmentParserMixin, Parser):
+    """
+    url_categories: url of the categories in api v2 (example: 'category': '/api/v2/touristiccontent_category/')


C'est peut-être plus "anglais" de dire "categories url", et donc la variable categories_url, mais rien d'obligatoire non plus

geotrek/common/tests/test_parsers.py

geotrek/trekking/parsers.py

numahell · 2022-08-03T15:10:09Z

geotrek/common/parsers.py

+        geom = GEOSGeometry(geom)
+        return geom
+
+    def next_row(self):


Est-ce que tu peux expliciter dans la docstring pourquoi on a besoin de surcharger cette méthode ?
et notamment l'usage de self.next_url

geotrek/common/parsers.py

numahell · 2022-08-04T08:30:43Z

geotrek/common/parsers.py

+    """
+    model = None
+    next_url = ''
+    url = None


Préciser quelque part que c'est mieux sans le / à la fin, sinon ça ne fonctionne pas

Ça peut être dans le message d'erreur par exemple

numahell · 2022-08-04T09:19:51Z

geotrek/trekking/parsers.py

+
+    def next_row(self):
+        self.next_url = f"{self.url}/api/v2/trek"
+        return super().next_row()


Peut-être qu'avec une variable endpoint_url ou un truc comme ça, y a moyen de ne pas être obligé de surcharger cette méthode ?

marcantoinedupre · 2022-08-04T09:34:16Z

J'ai eu une pensée pour le fait que le GTA aggrégateur et les GTAs aggrégés doivent être sur la même version du code. Comme il y aura forcément des moments où ce ne sera pas le cas (brièvement, par exemple pour les màj). Je me dis que l'avoir en tête au niveau de la gestion des erreurs est pas mal. Pour échouer gracieusement.

amandine-sahl · 2022-08-05T09:14:38Z

Quelques retours après tests sur des APIs en 2.85.0 et en 2.82.2

Globalement ça fonctionne bien et c'est assez simple à mettre en oeuvre

Messages :

Globalement il y a très peu de messages, ce qui est un peu dommage.
Quand une catégorie n'existe pas, le contenu n'est pas importé sans que ce soit signalé à l'utilisateur

Attachments :

Les médias accessibilités ne sont pas importés
Les thumbnails (favoris) ne ressortent pas
Les fichiers liés n'ont pas de miniature de prévisualisation
La licence n'est pas importée

Suppression des contenus :
Par défaut le parseur supprime tous les éléments de la catégorie du parseur. Pour aggréger des données de plusieurs sources il faut spécifier delete = False. Ce qui fait que les données ne sont pas supprimées. Hors si dans une source un contenu est supprimé il apparaît toujours.

Import différentiel :
Les parseurs traitent l'ensemble des données à chaque import pour des questions de performances il serait intéressant de ne récupérer que les données modifiées grâce au paramètre updated_after.

GeotrekSignageParser:

lors de l'import les fichiers liés ne sont pas importés avec un WARNING 1 geotrek.common.parsers Failed to fetch [...] after 3 times. Status code : 403.

GeotrekTouristicContentParser:

Fichiers liés non importés

LePetitTim · 2022-08-05T12:11:28Z

Par défaut le parseur supprime tous les éléments de la catégorie du parseur. Pour aggréger des données de plusieurs sources il faut spécifier delete = False. Ce qui fait que les données ne sont pas supprimées. Hors si dans une source un contenu est supprimé il apparaît toujours.

Tu as regardé avec les dernières modifications ?

amandine-sahl · 2022-08-05T13:47:25Z

Je viens de mettre à jour Geotrek et de re-tester et j'ai toujours le même comportement.
1 - Création d'un POI dans geotrekdemo
2 - Parseur d'import
3 - Suppression du POI dans geotrekdemo
4 - Parseur d'import
=> Le Poi est toujours dans la base d'aggrégation

class DemoGeotrekPOIParser(GeotrekPOIParser):
  delete = False
  url = "https://geotrekdemo.ecrins-parcnational.fr"

amandine-sahl · 2022-08-05T14:07:25Z

Autre point, je m'interroge sur la gestion des structures, pour le moment, si je ne me trompe pas elles ne sont pas prisent en compte.

LePetitTim · 2022-08-08T15:07:56Z

Je viens de mettre à jour Geotrek et de re-tester et j'ai toujours le même comportement. 1 - Création d'un POI dans geotrekdemo 2 - Parseur d'import 3 - Suppression du POI dans geotrekdemo 4 - Parseur d'import => Le Poi est toujours dans la base d'aggrégation
class DemoGeotrekPOIParser(GeotrekPOIParser):
  delete = False
  url = "https://geotrekdemo.ecrins-parcnational.fr"

En effet il y avait une erreur. La gestion de la suppression fonctionne dorénavant.

Add 'provider' field to aggregated models

LePetitTim force-pushed the add_aggregator_parser branch 3 times, most recently from 676e222 to c781dca Compare August 1, 2022 13:44

LePetitTim requested a review from a team August 1, 2022 15:39

numahell reviewed Aug 2, 2022

View reviewed changes

LePetitTim force-pushed the add_aggregator_parser branch from 0a81e3b to 6819ccd Compare August 2, 2022 13:11

numahell reviewed Aug 2, 2022

View reviewed changes

LePetitTim force-pushed the add_aggregator_parser branch 3 times, most recently from bdd9d28 to 6661937 Compare August 3, 2022 14:37

numahell reviewed Aug 3, 2022

View reviewed changes

numahell reviewed Aug 4, 2022

View reviewed changes

geotrek/common/parsers.py Show resolved Hide resolved

numahell reviewed Aug 4, 2022

View reviewed changes

Chatewgne approved these changes Aug 4, 2022

View reviewed changes

numahell reviewed Aug 4, 2022

View reviewed changes

LePetitTim force-pushed the add_aggregator_parser branch 2 times, most recently from c1883ac to 9ca79e7 Compare August 4, 2022 13:54

LePetitTim force-pushed the add_aggregator_parser branch 5 times, most recently from ce6985e to 983bfd6 Compare August 8, 2022 13:56

Chatewgne added 27 commits September 13, 2022 09:52

Add missing test data file

7a503fc

Fix tests

91ab39b

Fix tests

9cada5e

Set UUIDs of aggregated objects to both EID and UUID in database

a7b3ec0

Fix changelog

09e310d

Add 'provider' field to Path, Trail

cff574f

Add 'provider' field to Site, Course

e3eee3e

Add 'provider' field to SensitiveArea

785d7d8

Fix typo in serializer

9dd24f2

Fix use uuid as id in parsers

a3b725a

Fix use uuid as id in parsers tests

07b2cae

Fix use uuid as id in Trek parsers

e64c383

Change empty provider behaviour in parser

68192aa

Codestyle

21796fe

Remove print

9c3e4f8

Add end of files newlines

a369191

Replace 'eid_prefix' with 'provider' in GeotrekParser

b16af28

Improve error message in Tour parsing

2ef70b5

Add missing provider filters

9a6058d

Add db index on providers

84d8af6

Fix Outdoor Managers

9be7899

Fix tests with new queries

45e4f9a

Start documenting Parser attributes

3684194

Alter proper table in SQL templates for Signage

0ac86f1

Fix test should not depend on list order

c92eec9

Merge pull request #3226 from GeotrekCE/delete_by_provider

19f6b09

Add 'provider' field to aggregated models

Add warning and test for missing Tour parent trek

b72044e

Chatewgne merged commit 05ef0ef into master Sep 20, 2022

submarcos deleted the add_aggregator_parser branch September 30, 2022 08:59

camillemonchicourt mentioned this pull request Jan 10, 2023

[Retour Journée Geotrek] Aggregation Geotrek #2824

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add aggregator parser #3196

Add aggregator parser #3196

LePetitTim commented Jul 28, 2022

cypress bot commented Jul 28, 2022 •

edited

Loading

codecov bot commented Jul 28, 2022 •

edited

Loading

numahell left a comment

numahell Aug 2, 2022

numahell Aug 2, 2022

numahell left a comment

numahell Aug 3, 2022

numahell Aug 3, 2022

numahell Aug 4, 2022

numahell Aug 4, 2022

numahell Aug 4, 2022

marcantoinedupre commented Aug 4, 2022

amandine-sahl commented Aug 5, 2022 •

edited

Loading

LePetitTim commented Aug 5, 2022

amandine-sahl commented Aug 5, 2022 •

edited

Loading

amandine-sahl commented Aug 5, 2022

LePetitTim commented Aug 8, 2022

	parser.add_argument('filename', nargs="?", help='Optional file which will be use to feed database')
	parser.add_argument('filename', nargs="?", help='Optional file used to feed database')

Add aggregator parser #3196

Add aggregator parser #3196

Conversation

LePetitTim commented Jul 28, 2022

cypress bot commented Jul 28, 2022 • edited Loading

Test summary

Run details

codecov bot commented Jul 28, 2022 • edited Loading

Codecov Report

numahell left a comment

Choose a reason for hiding this comment

numahell Aug 2, 2022

Choose a reason for hiding this comment

numahell Aug 2, 2022

Choose a reason for hiding this comment

numahell left a comment

Choose a reason for hiding this comment

numahell Aug 3, 2022

Choose a reason for hiding this comment

numahell Aug 3, 2022

Choose a reason for hiding this comment

numahell Aug 4, 2022

Choose a reason for hiding this comment

numahell Aug 4, 2022

Choose a reason for hiding this comment

numahell Aug 4, 2022

Choose a reason for hiding this comment

marcantoinedupre commented Aug 4, 2022

amandine-sahl commented Aug 5, 2022 • edited Loading

LePetitTim commented Aug 5, 2022

amandine-sahl commented Aug 5, 2022 • edited Loading

amandine-sahl commented Aug 5, 2022

LePetitTim commented Aug 8, 2022

cypress bot commented Jul 28, 2022 •

edited

Loading

codecov bot commented Jul 28, 2022 •

edited

Loading

amandine-sahl commented Aug 5, 2022 •

edited

Loading

amandine-sahl commented Aug 5, 2022 •

edited

Loading