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

Admin : ajout d'un filtre et affichage du updated_at pour GPS #4310

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

celine-m-s
Copy link
Collaborator

@celine-m-s celine-m-s commented Jun 26, 2024

🤔 Pourquoi ?

Pour mieux mesurer notre impact.

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ Comment tester

Aller dans l'admin de Django puis dans Relation ou Groupe de suivi.

💻 Captures d'écran

image

image

@celine-m-s celine-m-s added the ajouté Ajouté dans le changelog. label Jun 26, 2024
@celine-m-s celine-m-s self-assigned this Jun 26, 2024
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch from 0697699 to 7d4a544 Compare June 26, 2024 17:49
@celine-m-s celine-m-s added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Jun 26, 2024
Copy link

Copy link

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

itou/gps/models.py Outdated Show resolved Hide resolved
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch 2 times, most recently from 3be73e6 to 75d2bd0 Compare June 27, 2024 14:16
@celine-m-s celine-m-s requested a review from tonial June 27, 2024 14:17
itou/gps/admin.py Outdated Show resolved Hide resolved
itou/gps/admin.py Outdated Show resolved Hide resolved


def _update_follow_up_groups_created_in_bulk(apps, schema_editor):
FollowUpGroup = apps.get_model("gps", "FollowUpGroup")
Copy link
Contributor

Choose a reason for hiding this comment

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

Attention, j'ai peut que la migration dire trop longtemps vu le nombre d'objets à vérifier.
à chaque appel à groups[:10000] django va refaire une requête SQL et donc parcourir les objets un par un pour regarder si la date fonctionne.

Il est possible que ce soit rapide vu que presque tous sont dans ce cas là, mais je n'exclue pas que ça puisse être méga long (j'ai eu le soucis pour remplir les JobApplication.processed_at cette semaine)

Je vois 3 possibilités :

  • lancer le remplissage depuis une fastmachine avant de déployer la migration gps.0003
  • ajouter un index sur created_at à la main via psql avant (et le retirer après)
  • ajouter un index sur created_at via django avant (et le retirer après)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bonne idée ! J'ai ajouté un index sur created_at. Peux-tu tester en local ?

Copy link
Contributor

Choose a reason for hiding this comment

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

c'est top : 1 minute 30 secondes en local
sans l'index ça prend plutôt 2 minutes donc le gain semble en effet faible, mais au moins on est tranquille au cas où la bdd a du mal (comme ça été le cas pour la création des groupes)

itou/gps/migrations/0003_auto_20240627_1453.py Outdated Show resolved Hide resolved
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch from 75d2bd0 to 7c02b70 Compare June 28, 2024 13:22
@celine-m-s celine-m-s requested a review from tonial June 28, 2024 13:23
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch 2 times, most recently from dbb35b1 to 46652d4 Compare June 28, 2024 13:26
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch from 46652d4 to 411cb06 Compare June 28, 2024 15:14
Copy link
Contributor

Choose a reason for hiding this comment

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

Un nom de migration plus clair peut-être ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fait !

@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch from 411cb06 to a0bc470 Compare July 2, 2024 10:08
@celine-m-s celine-m-s requested a review from xavfernandez July 2, 2024 10:09
@celine-m-s celine-m-s removed the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Jul 2, 2024
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch from a0bc470 to 7848153 Compare July 2, 2024 12:55
@celine-m-s celine-m-s force-pushed the celinems/admin-filter branch from 7848153 to 75e908b Compare July 2, 2024 14:21
@celine-m-s celine-m-s enabled auto-merge July 2, 2024 14:21
@celine-m-s celine-m-s added this pull request to the merge queue Jul 2, 2024
Merged via the queue into master with commit 4cf9070 Jul 2, 2024
11 checks passed
@celine-m-s celine-m-s deleted the celinems/admin-filter branch July 2, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ajouté Ajouté dans le changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants