Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(all) : Récupérer les données géo depuis l'API découpage administratif #282

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

vperron
Copy link
Contributor

@vperron vperron commented Aug 30, 2024

Ceci permet entre autres de:

  • récupérer les DOM/TOM
  • récupérer l'intégralité des codes EPCIS, Départements, Régions à jour
  • les communes ainsi que les arrondissements
  • ne plus dépendre ni d'admin_express, ni des exports de l'INSEE
  • de ne pas avoir un import séparé dans le pipeline et dans l'API
  • passer à un modèle de détection de distance par rapport au centre de la ville et non pas son contour, donc plus performant et avec peu de perte de précision (pas encore réalisé)
  • bien d'autres joyeusetés à venir en termes de validation, cohérence, etc

@vperron vperron requested a review from hlecuyer August 30, 2024 16:27
@vperron vperron self-assigned this Aug 30, 2024
@vmttn
Copy link
Contributor

vmttn commented Sep 2, 2024

Je pense qu'il y a moyen de simplifier 🙊

  1. On peut se passer du dump des extractions dans s3. Contrairement aux fournisseurs de données, le niveau de service de cette api devrait être bonne et les données relativement stables, en tout cas il n'y a pas de raisons de penser le contraire
  2. Séparer clairement les transformations de l'extraction et utiliser dbt pour faire les transfo et le nettoyage des données. L'extraction peut se résumer à quelques appel, du json chargé dans une colonne jsonb, nettoyée par dbt et passée au format tabulaire avec des tests
  3. Je ne peux pas m'empêcher de penser qu'on devrait charger les données également dans l'api à partir de l'api découpage. Les données structures/services sont produites par notre pipeline ce qui justifie leur stockage régulier sur s3. Le risque d'incohérence me parait franchement minime et une simple vérification au moment de l'import de nos données devrait suffire à écarter les quelques incohérences qui pourraient se produire si on n'a pas pris le soin de mettre à jour les données découpages dans l'api et la pipeline simultanément

@vperron
Copy link
Contributor Author

vperron commented Sep 2, 2024

  1. OK, ça m'ira toujours de simplifier ! J'ai toujours ce conflit cohérence avec le reste du code VS traitement spécifique.
  2. OK aussi, ça me va bien (d'où le wip)
  3. A moitié convaincu, car avec ta solution:
  • On a besoin d'un second script d'extraction (similaire mais assez différent, p.ex merger les arrondissements reste valide mais le traitement, création de la table et traitement éventuel sur les colonnes seront en Python/Alembic et non pas en DBT)
  • N'élimine pas le risque d'incohérence, au contraire on en introduit un. OK il suffit d'un check mais ça ajoute un check qui aurait pu être prétesté en pipeline.
  • On n'est pas à l'abri que demain l'API ait besoin d'autres tables venant de notre pipeline pour enrichir les données (voire enrichir les données de commune)
    ... et encore une fois pour moi ça permet de ne conserver qu'une seule méthode pour charger toutes les données de l'API en un seul coup, ce qui le rend plus simple, solide et facile à maintenir.

@vperron vperron force-pushed the vperron/sync-cities branch 2 times, most recently from 8d2173c to 7988635 Compare September 3, 2024 08:20
@vperron vperron marked this pull request as ready for review September 3, 2024 08:21
@vperron vperron requested a review from vmttn as a code owner September 3, 2024 08:21
@vperron vperron marked this pull request as draft September 3, 2024 08:22
@vperron vperron marked this pull request as ready for review September 3, 2024 15:52
@vperron
Copy link
Contributor Author

vperron commented Sep 3, 2024

A priori, fait les ajustements (grosse maille) que tu voulais voir @vmttn .

pipeline/dags/sync_cities.py Outdated Show resolved Hide resolved
pipeline/dags/sync_cities.py Outdated Show resolved Hide resolved
pipeline/dags/sync_cities.py Outdated Show resolved Hide resolved
pipeline/dags/sync_cities.py Outdated Show resolved Hide resolved
@vperron
Copy link
Contributor Author

vperron commented Sep 10, 2024

Remarques traitées et commandes d'import des communes restaurée coté API.

That pulls data from the API "decoupage administratif".
The generated tables (admin_express_xxx) are unused and do not even
exist in our production database since a long, long time.
@vmttn vmttn merged commit 3367584 into main Sep 13, 2024
8 of 9 checks passed
@vmttn vmttn deleted the vperron/sync-cities branch September 13, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants