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

PC-33895 chronicles route #15900

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

cnormant-pass
Copy link
Contributor

But de la pull request

Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-33895
Route des chroniques d'une offre

Vérifications

  • J'ai écrit les tests nécessaires

Comment on lines 119 to 120
.options(joinedload(Offer.chronicles))
.options(joinedload(Offer.product).joinedload(Product.chronicles))
Copy link
Contributor

Choose a reason for hiding this comment

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

Attention, si on a n chroniques sur l'offre + p chroniques sur le produit, alors la base de données renverra n*p lignes. Cette requête peut devenir gourmande en ressources en cas de très nombreuses chroniques (200 sur chaque => 40.000 lignes SQL).

Si le nombre de chroniques n'est pas maîtrisé, mais que seul le nombre de chroniques publiées l'est, peut-être une bonne chose serait-elle de ne faire d'outerjoin que sur les chroniques publiées, qui seraient filtrées dès la base de données, sans rapatrier celles non publiées, dont on n'a pas besoin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok j'ai changé de stratégie pour simplifier le code
Je fais une query de plus mais je suis sur d'avoir une réponse avec peu de lignes
C'est dans le dernier commit, je squash ça correctement après les reviews :)

api/tests/routes/native/v1/offers_test.py Outdated Show resolved Hide resolved
@cnormant-pass cnormant-pass force-pushed the PC-33895-chronicles-route branch from 4808c2f to 0acbb7c Compare January 17, 2025 17:27
Copy link
Contributor

mypy cop report: 459 (master) ↗ 460 (your branch)

Copy link
Contributor

Visit the preview URL for this PR (updated for commit 0acbb7c):

https://pc-pro-testing--pr15900-pc-33895-chronicles-bl73ifws.web.app

(expires Sun, 19 Jan 2025 17:29:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1

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.

3 participants