Skip to content

Conversation

@MisterOryon
Copy link
Contributor

This update includes request_distribution_ids in the Bench API response, allowing clients to access associated request distribution IDs. Test coverage has been added to verify the presence and correctness of this new data field.

This update includes request_distribution_ids in the Bench API response, allowing clients to access associated request distribution IDs.
Test coverage has been added to verify the presence and correctness of this new data field.
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2025

@MisterOryon MisterOryon merged commit 95b4fbe into master Jan 8, 2025
6 checks passed
@MisterOryon MisterOryon deleted the feat/bench_api_v2 branch January 8, 2025 10:07
Comment on lines +7 to +9
field :request_distribution_ids do |bench|
bench.request_distributions.pluck(:id)
end
Copy link
Member

Choose a reason for hiding this comment

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

C'est effectivement une bonne manière d'ajouter les IDs de nos request_distributions sur nos bench :)

En revanche, on crée ainsi une N+1 ; ce qu'il faut comprendre, c'est qu'en renvoyant une liste de benches, on va pour chacun d'entre eux exécuter une requête SQL pour récupérer les IDs des request_distribution de notre bench.

Si on renvoie 10 benches, on va donc exécuter une requête SQL pour récupérer la liste des 10 benches, et pour chacun d'entre eux, faire une requête SQL pour faire un SELECT request_distributions.id FROM ..., donc 11 requêtes en tout (d'où le terme N+1).

Pour éviter ça, dans notre controller, on pourrait déjà charger la liste des request_distributions de tous nos bench, et ainsi ne faire que 2 requêtes en tout, indépendamment de la taille de notre liste de benches :) Pour ça, on peut utiliser le includes par exemple.

En remplaçant donc la ligne benches = policy_scope(@greenhouse.benches) dans notre controller par benches = policy_scope(@greenhouse.benches).includes(:request_distributions), on corrige une N+1 🚀

Oh, et pour finir, petit sucre syntaxique sympa avec rails : à la place du bench.request_distributions.pluck(:id) ici présent, vous pouvez écrire bench.request_distribution_ids 👌

Copy link
Contributor Author

@MisterOryon MisterOryon Jan 27, 2025

Choose a reason for hiding this comment

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

Merci pour ton retour. Effectivement, cela va poser problème lorsqu'il y aura beaucoup de données à gérer.
Je vais suivre tes recommandations et changer cela ce week-end :)

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.

4 participants