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

🚀 CLUSTERING - v2: sélection, norma, suggestions #1200

Merged
merged 12 commits into from
Jan 16, 2025

Conversation

maxcorbeau
Copy link
Contributor

🚀 CLUSTERING - v2: sélection, norma, suggestions

🟢 cette PR ne fait que de la lecture/affichage pour que à @chrischarousset de commencer à jouer avec la UI Airflow 🟢

  • 💡 quoi: on continue sur le DAG de suggestions de clustering acteurs
  • 🎯 pourquoi: permettre à @chrischarousset de continuer à tester
  • 🤔 comment:
    • intégration django suite à 📦 Intégration django -> airflow #1189
      • Airflow se base sur le modèle acteur
      • des propriétés calculérs/dérivées on été ajoutées
    • sélection: fonctionalités étendues après discussions avec @chrischarousset
    • normalization: ajout de ce qui était utilisé dans le script de clustering local
    • suggestions: ajout de ce qui était utilisé dans le script de clustering local

➡️ A faire pour les prochaines PR

  • Explications d'ensemble: pour la dernière PR une fois le code stabilisé je rentrerai plus en détails dans les explications du clustering en lui même
  • 1 test end-to-end pour le DAG vu sa complexité
  • Améliorer les tests liés à django avec du mock de DB. @fabienheureux et @kolok : voir test_cluster_acteurs_db_data_read_acteurs.py: je cherche à mocker quelques entrées Acteurs mais ce que j'ai ne marche pas pour l'instant, pouvez-vous me donner quelques pointeurs?
  • Améliorer les tests de chaque tâche avec des scénarios concrets établis par @chrischarousset
  • Ecriture des suggestions en DB: une fois qu'on a Début de la refactorisation de l'interface dags/validations #1174 terminé

Airflow

⚠️ Complexité des paramètres UI

Il y a 15 paramètres UI airflow, donc dans la pratique des centaines de configurations possibles (je compte pas 2^15 parce qu'il y a des scénarios qu'on autorise pas, comme par exemple on force certains champs à être remplis). Donc l'objectif pour la prochaine PR est de créer une dataclass de params pour:

  • améliorer la robustesse de la validation de config avec typing/décorations et tests qui vont bien
  • utiliser la config plus facilement à travers les différentes tâches airflow (une fois valider par la dataclass on peut faire du params.{name} avec autocomplete/validation IDE ce qu'on a pas pour l'instant sur sa forme dict)

✋ AirflowSkipException

J'ai commencé à implémenté ce statut qui permet de voir (en rose) les tâches qui ne produisent rien, j'aimerais trouver le temps de soumettre une petite doc pour formaliser les règles d'usage des différents statuts:

image

🖼️ UI

Qui doit ressembler à cela:

screencapture-localhost-8080-dags-cluster-acteurs-suggestions-trigger-2025-01-13-17_17_29

@maxcorbeau maxcorbeau requested a review from a team as a code owner January 13, 2025 16:31
@maxcorbeau maxcorbeau requested review from kolok and fabienheureux and removed request for a team January 13, 2025 16:31
@maxcorbeau maxcorbeau force-pushed the acteur_clustering_v2 branch from 2c659fb to 75302d6 Compare January 14, 2025 05:47
@maxcorbeau maxcorbeau enabled auto-merge (squash) January 14, 2025 06:42
airflow-requirements.in Outdated Show resolved Hide resolved
dags/utils/django.py Show resolved Hide resolved
dags/utils/django.py Outdated Show resolved Hide resolved
dags/utils/django.py Outdated Show resolved Hide resolved
@maxcorbeau
Copy link
Contributor Author

dernier commit (je l'éspère) pour pouvoir merger et permettre à @chrischarousset de tester sélection + normalisation

image

Copy link
Contributor

@kolok kolok left a comment

Choose a reason for hiding this comment

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

Encore quelques remarques mineurs mais rien de bloquant

@maxcorbeau maxcorbeau merged commit 0b12b44 into main Jan 16, 2025
11 checks passed
@maxcorbeau maxcorbeau deleted the acteur_clustering_v2 branch January 16, 2025 08:21
Copy link

Copy link

Copy link

kolok pushed a commit that referenced this pull request Jan 16, 2025
* clustering acteurs v2: sélection, norma, suggestions (pas écriture)

* clustering: fix requirements - essai 1

* clustering: fix requirements - essai 2

* Réparation du test sur airflow_params_dropdown
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.

2 participants