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

Preuve de concept de l'utilisation de DBT #1357

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Preuve de concept de l'utilisation de DBT #1357

wants to merge 6 commits into from

Conversation

kolok
Copy link
Contributor

@kolok kolok commented Feb 14, 2025

Description succincte du problème résolu

N'oublier pas de taguer : bug, enhancement, documentation, technical, dependencies

🗺️ contexte: Recherche de performance car le calcule des acteurs à afficher n'est pas fou fou

💡 quoi:

  1. Créer la table exhaustive des acteurs en utilisant DBT et des nouvelles tables
  2. Correction des données VueActeur qui dans certains cas pouvait être dupliqué
  3. Créer la table des acteurs à afficher

🎯 pourquoi: Gagner en performance, testatbilité

🤔 comment:

  1. Création de table qfdmo_vue… via DBT et ajout des indexes et des contraintes pour garantir performance et consistance
  2. Création des tests des tables créées via la définition dans schema.yml
  3. Orchestration de DBT avec Airflow : dbt run puis dbt test

Exemple résultats / UI / Data

Création des tables :

CleanShot 2025-02-14 at 15 11 10

Tests des tables crées

CleanShot 2025-02-14 at 15 10 56

Résultat = 90 ms pour afficher un acteur relativement complexe en local vs entre 10 et 20 en prod

CleanShot 2025-02-14 at 15 14 27

Auto-review

Les trucs à faire avant de demander une review :

  • J'ai bien relu mon code
  • La CI passe bien
  • En cas d'ajout de variable d'environnement, j'ai bien mis à jour le .env.template
  • J'ai ajouté des tests qui couvrent le nouveau code

✅ Reste à faire (PR en cours)

  • Ajouter un premier test de cohérence
  • Ajouter les tests dans la CI
  • Prise en charge des valeurs "empty" pour chacun des champs lors de la compilation des corrections
  • Ré-architecturer selon les concepts
  • Documentation à mettre à jour

📆 A faire (prochaine PR)

  • Remplacer / remplir les tables/views displayedacteur et vueacteur par les tables compilés

@kolok kolok added the enhancement New feature or request label Feb 14, 2025
@kolok kolok force-pushed the dbt_poc branch 4 times, most recently from 9ae8d24 to 46dce89 Compare February 14, 2025 14:54
dbt-core==1.9.2
dbt-postgres==1.9
Copy link
Contributor Author

@kolok kolok Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ajout des dépendance à dbt, le reste des modifications c'est un tri alphabétique des lignes

Comment on lines +46 to +49
WORKDIR /opt/airflow/dbt
USER 0
RUN chown -R ${AIRFLOW_UID:-50000}:0 /opt/airflow/dbt
USER ${AIRFLOW_UID:-50000}:0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A revoir, redonner les droits à root pour faire le chown, c'est pas ouf :)

ENV DBT_PROFILES_DIR=/opt/airflow/dbt
ENV DBT_PROJECT_DIR=/opt/airflow/dbt

RUN dbt deps
Copy link
Contributor Author

@kolok kolok Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A creuser si on peut installer ces dépendances avec pip, ça éviterait d'avoir des dépendances gérées ailleurs

Update : à priori, ce n'est pas possible

Comment on lines +35 to +57
POSTGRES_HOST=lvao-db
POSTGRES_PORT=5432
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valeur pour l'ececution dans le docker

dans mon .env pour le local, j'ai mis

POSTGRES_HOST=localhost
POSTGRES_PORT=6543

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A documenter

@maxcorbeau
Copy link
Contributor

  • très bien: on s'oriente petit à petit vers l'archi idéale je pense 👏
  • prochaine étape: on simplifie pour avoir:
    • 1 x modèle/table d'état acteur (au lieu de 3x Base, Revision, Displayed)
    • 1 x modèle/table de mutation/suggestion (celle sur laquelle on bosse dans data/)
    • 1 x SDK/API pour centraliser tous les changements d'état (au lieu N x pour l'instant)
    • Le reste c'est des vues SQL gérées par DBT (ex: acteurs -> acteurs_actif -> acteurs_sur_la_carte etc...)

Et là je pense qu'on sera bien 🦾

@maxcorbeau
Copy link
Contributor

maxcorbeau commented Feb 17, 2025

Le problème majeur de cette PR pour moi c'est qu'on réspècte pas l'approche par couche de dépendences: https://docs.getdbt.com/best-practices/how-we-structure/1-guide-overview

  • Dans cette PR on a les modèles organisés par finalité (carte_acteurs, exhaustive_acteurs etc...)
  • Alors que le best practice c'est d'organiser par couche de construction (base, staging, intermediate etc...)

La raison pour les couche de construction:

  • Regroupement des logiques: ex: base = plugger les sources, staging = rendre donnée de base utilisable (e.g. filtre, nettoyage), intermediate = on applique la logique métier au niveau modèle, marts = on combine les modèles intermédiaire pour des logiques cross-modèles
  • Lineage simple et debuggable: ex: on sait que intermediate ne peut dépendre que de staging, que marts ne peux dépendre que de intermediate etc...
  • Eviter les boucles: avec des modèles qui dépendent les uns des autres au même niveau
  • Suivre les standards: embarquer les nouveaux ingé + pouvoir échanger avec la communauté plus facilement car on fait pas du full custom

Happy d'avoir une session brainstorm et d'aider à créer la stucture initiale DBT

@kolok kolok force-pushed the dbt_poc branch 4 times, most recently from f08a29c to 894221b Compare February 18, 2025 12:43
@kolok
Copy link
Contributor Author

kolok commented Feb 18, 2025

  • très bien: on s'oriente petit à petit vers l'archi idéale je pense 👏

    • prochaine étape: on simplifie pour avoir:

      • 1 x modèle/table d'état acteur (au lieu de 3x Base, Revision, Displayed)
      • 1 x modèle/table de mutation/suggestion (celle sur laquelle on bosse dans data/)
      • 1 x SDK/API pour centraliser tous les changements d'état (au lieu N x pour l'instant)
      • Le reste c'est des vues SQL gérées par DBT (ex: acteurs -> acteurs_actif -> acteurs_sur_la_carte etc...)

Et là je pense qu'on sera bien 🦾

Merci pour le retour

J'ai fait une proposition de découpage par concept que j'ai décrit là : docs/explications/data/dbt/flux-dbt.md

Pas mis en place dans cette PR.

La difficulté c'est que selon la finalité, l'étape 2 (filtrage) est différente et il n'est pas possible de faire le filtrage plus tard
Je n'ai pas encore trouvé une manière élégante de réutiliser les même models pour toutes les finalité mais après filtrage

@kolok kolok marked this pull request as ready for review February 18, 2025 12:47
@kolok kolok requested a review from a team as a code owner February 18, 2025 12:47
@kolok kolok requested review from fabienheureux and maxcorbeau and removed request for a team February 18, 2025 12:47
Copy link
Contributor

@maxcorbeau maxcorbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'approuve pour avancer mais

Copy link
Contributor

@maxcorbeau maxcorbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'approuve pour te permettre d'avancer. On a notre atelier du 3 mars pour établir la structure & principes de notre stack DBT

@kolok kolok force-pushed the dbt_poc branch 6 times, most recently from 92c0d5a to 87fdb5b Compare March 6, 2025 15:16
@kolok
Copy link
Contributor Author

kolok commented Mar 6, 2025

Les tests sont concluants:

qfdmo=# SELECT count(distinct(identifiant_unique)) 
qfdmo-# FROM qfdmo_displayedacteur as a
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice AS ps
qfdmo-#  ON ps.acteur_id = a.identifiant_unique
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice_sous_categories AS ps_sscat ON ps_sscat.displayedpropositionservice_id = ps.id;

 count  
--------
 402471
(1 row)

qfdmo=# 
qfdmo=# SELECT count(*) FROM marts_carte_acteur;
 count  
--------
 402471
(1 row)

qfdmo=# SELECT count(distinct(ps.id))
qfdmo-# FROM qfdmo_displayedpropositionservice AS ps
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice_sous_categories AS ps_sscat ON ps_sscat.displayedpropositionservice_id = ps.id;
 count  
--------
 473947
(1 row)

qfdmo=# SELECT count(*) FROM marts_carte_propositionservice;
 count  
--------
 473947
(1 row)

qfdmo=# SELECT count(*) FROM qfdmo_displayedpropositionservice_sous_categories;
  count  
---------
 1326912
(1 row)

qfdmo=# SELECT count(*) FROM marts_carte_propositionservice_sous_categories;
  count  
---------
 1326912
(1 row)

qfdmo=# 
qfdmo=# SELECT count(distinct(l.id))
qfdmo-# FROM qfdmo_displayedacteur_labels as l
qfdmo-# INNER JOIN qfdmo_displayedacteur as a ON a.identifiant_unique = l.displayedacteur_id
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice AS ps ON ps.acteur_id = a.identifiant_unique 
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice_sous_categories AS ps_sscat ON ps_sscat.displayedpropositionservice_id = ps.id;
 count 
-------
 19602
(1 row)

qfdmo=# SELECT count(*) FROM marts_carte_acteur_labels;                                                
 count 
-------
 19602
(1 row)

qfdmo=# 
qfdmo=# SELECT count(distinct(l.id))
qfdmo-# FROM qfdmo_displayedacteur_acteur_services as l
qfdmo-# INNER JOIN qfdmo_displayedacteur as a ON a.identifiant_unique = l.displayedacteur_id
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice AS ps ON ps.acteur_id = a.identifiant_unique 
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice_sous_categories AS ps_sscat ON ps_sscat.displayedpropositionservice_id = ps.id;
 count  
--------
 408304
(1 row)

qfdmo=# SELECT count(*) FROM marts_carte_acteur_acteur_services;
 count  
--------
 408304
(1 row)

qfdmo=# SELECT count(distinct(l.id))
qfdmo-# FROM qfdmo_displayedacteur_sources as l
qfdmo-# INNER JOIN qfdmo_displayedacteur as a ON a.identifiant_unique = l.displayedacteur_id
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice AS ps ON ps.acteur_id = a.identifiant_unique 
qfdmo-# INNER JOIN qfdmo_displayedpropositionservice_sous_categories AS ps_sscat ON ps_sscat.displayedpropositionservice_id = ps.id;
 count  
--------
 417712
(1 row)

qfdmo=# SELECT count(*) FROM marts_carte_acteur_sources;
 count  
--------
 417712
(1 row)

Le dags s'execute correctement:

CleanShot 2025-03-06 at 16 12 37@2x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants