Correction du dag CMA #989
Conversation
kolok
requested review from
fabienheureux and
maxcorbeau
and removed request for
a team
J'ai ajouté le seul test que je pouvais faire sans ajouter de dépendance des tests à la base de donnée :) |
kolok
force-pushed
the
cma_get_from_source_code
branch
from
e2be736
to
cfcafb0
Compare
There was a problem hiding this comment.
@kolok : OK pour le merge, quelques remarques sur la déclaration/typing + potentielle amélioration de perf pour get_id_from_code
dags/config/db_mapping.json
Outdated
| @@ -68,7 +68,8 @@ | |||
| "materiel informatique" | |||
| ], | |||
| "téléphones, smartphones": ["smartphone, tablette et console"], | |||
| "vêtements, textiles": ["vetement", "linge de maison"] | |||
| "vêtements, textiles": ["vetement", "linge de maison"], | |||
| "autres":[] | |||
There was a problem hiding this comment.
@kolok : même remarque que pour mapping des colonnes, je privilégierais un None plus explicite que [] pour clairement indiquer l'intention de ne pas mapper (et non pas un oubli potentiel de définir la correspondance)
dags/utils/dag_eo_utils.py
Outdated
| @@ -275,14 +275,21 @@ def serialize_to_json(**kwargs): | |||
|
|
|||
|
|
|||
| def read_acteur(**kwargs): | |||
| source_code = kwargs["params"].get("source_code", []) | |||
There was a problem hiding this comment.
@kolok : dans cma.py tu définis le source_code comme chaîne:
"source_code": "CMA - Chambre des métiers et de l'artisanat"
Et là tu changes de type vers de la liste par défaut.
Normalement des librairies comme https://github.com/python/mypy retournent des exceptions pour le changement de type, malheureusement on passe par des accès indirectes de dict donc je pense que c'est mort pour le typing.
Piste d'amélioration: définir un modèle (ex: dataclass/pydantic) pour le kwargs["params"] qui contient nos paramètres custom avec contraintes de type.
| df["produitsdechets_acceptes"] = df.apply(combine_categories, axis=1) | ||
| df["source_id"] = get_id_from_code( |
There was a problem hiding this comment.
@kolok : à revoir, mais get_id_from_code fait le travail 2x: 1ère fois pour le test any et une 2ème fois pour récupérer une valeur, potentiellement moyen de faire en 1 fois.
There was a problem hiding this comment.
un outil que j'aime bien c'est https://github.com/ionelmc/pytest-benchmark:
- il s'intègre avec
pytest(pas un gros framework lourd de stress test à rajouter donc c'est pratique) - on peut créer l'arborescence
tests/avectests/unit/(unit tests) ettests/perf(performance optimisation) - dès qu'on écrit des fonctions on vient compléter (on garde l'historique des fonctions)
tests/perfpour:- démontrer qu'on améliore les perfs
- documenter ce qui marche mieux/moins bien pour éduquer et réutiliser ce qui marche mieux
There was a problem hiding this comment.
A chaque fois qu'on utilise la fonction get_id_from_code c'est pour récupérer les iddepuis le code, que penses-tu de transformer les dataframes en dict(code -> id) dès l'appel à la base de données ?
En attendant de transformer les données en DB et de récuprer les ID via des JOIN
There was a problem hiding this comment.
Oui un dict lookup sera toujours plus rapide qu'une évaluation de dataframe c'est vrai
kolok
force-pushed
the
cma_get_from_source_code
branch
from
cfcafb0
to
e32d948
Compare
Description succincte du problème résolu
Le DAG de la CMA ne fonctionne plus depuis une optimisation lors de la récupération de des acteurs en base de données
Type de changement :
Auto-review
Les trucs à faire avant de demander une review :
.env.templateComment tester
En local / staging :