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.

Sign up for GitHub

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

Tech : Correction d'un test bagottant #5311

Closed
wants to merge 1 commit into from
Closed

Tech : Correction d'un test bagottant #5311

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/metabase/management/test_populate_metabase_emplois.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def test_populate_job_seekers():
1,
1,
1,
job_application_2.eligibility_diagnosis.created_at.date(),
timezone.localdate(job_application_2.eligibility_diagnosis.created_at, timezone=datetime.UTC),
Copy link
Contributor

Choose a reason for hiding this comment

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

Pourquoi UTC ? Si on veut la date de création du diag, cela me semble plus logique de convertir le timestamp en se basant sur Europe/Paris non ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Venant du modèle, created_at est un datetime.datetime avec la TZ "Europe/Paris".
De ce que j'ai compris là où ça part en eau de boudin c'est après, la table ainsi que les données sont créées directement via psycopg ce qui fait qu'a priori la TZ n'est pas configurée au niveau de la connexion (ce qui est le cas pour django.db.connection qu'on utilise pour lire les données) donc on est en UTC mais on envois un timestamptz donc PG fait la conversion :

[email protected]:itou=# SHOW TIMEZONE;
 Etc/UTC
[email protected]:itou=# SELECT '2024-11-06 00:30:00+00'::timestamptz::date;
 2024-11-06
[email protected]:itou=# SELECT '2024-11-06 00:30:00+01'::timestamptz::date;
 2024-11-05

[email protected]:itou=# SHOW TIMEZONE;
 Europe/Paris
[email protected]:itou=# SELECT '2024-11-06 00:30:00+00'::timestamptz::date;
 2024-11-06
[email protected]:itou=# SELECT '2024-11-06 00:30:00+01'::timestamptz::date;
 2024-11-06

On se retrouve donc avec la date UTC et non pas Europoe/Paris stockée dans la DB, vu que beaucoup de tests du fichier mentionne explicitement la TZ UTC au moment des comparaisons c'est (j'espère) le comportement voulu, et quitte à faire une erreur autant qu'elle soit (à peu près) la même partout, mais oui c'est pas ouf...

Copy link
Contributor

@xavfernandez xavfernandez Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ça ne me choque pas de stocker la date en UTC.
Mais quand on demande à quelle date a été créé un diagnostic, cela me semble plus logique de répondre en utilisant la TZ Europe/Paris. Mais du coup l'erreur originelle vient de

les-emplois/itou/metabase/tables/job_seekers.py

Line 297 in b47fe4a

"fn": lambda o: getattr(get_latest_diagnosis(o), "created_at", None),

en envoyant une datetime dans une colonne de type date sans faire de conversion explicite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Si la logique était de stocker les dates en Europe/Paris, alors oui.
Par contre cette situation et conversion arrive aussi pour plein d'autres champs donc la logique, par voie de fait, est actuellement de stocker les dates en UTC.
Je doute qu'on se lance dans ce changement/correction à plus ou moins court terme car un certain nombre de ces champs ne devraient pas être tronqué en fait, mais derrière ça veux dire tout adapter dans les modèles DBT et metabase 😓.

job_application_2.eligibility_diagnosis.expires_at,
0,
job_application_2.eligibility_diagnosis.author_prescriber_organization.id,
Expand Down
Loading