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

chore: mise en cache en environnement de developpement #862

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

vincentporte
Copy link
Contributor

@vincentporte vincentporte commented Dec 17, 2024

🐗 problème rencontré

CI

🦺 DatabaseCache => tests ok

pytest local :

🦺 DatabaseCache => tests KO 🧧

FAILED lacommunaute/forum/tests/tests_views.py::ForumViewTest::test_filtered_queryset_on_tag - AssertionError: 32 != 20 : 32 queries executed, 20 expected
FAILED lacommunaute/forum/tests/tests_views.py::TestDocumentationCategoryForumContent::test_numqueries_on_tags - Failed: Expected to perform 19 queries but 31 were done (add -v option to show queries)
FAILED lacommunaute/forum_conversation/tests/tests_views.py::TopicViewTest::test_numqueries - AssertionError: 51 != 39 : 51 queries executed, 39 expected
FAILED lacommunaute/stats/tests/tests_views.py::TestForumStatView::test_num_queries - Failed: Expected to perform 7 queries but 19 were done (add -v option to show queries)
FAILED lacommunaute/forum/tests/tests_views.py::ForumViewTest::test_queries - AssertionError: 34 != 22 : 34 queries executed, 22 expected
FAILED lacommunaute/forum_upvote/tests/test_upvotelistview.py::test_numqueries - Failed: Expected to perform 37 queries but 49 were done (add -v option to show queries)
FAILED lacommunaute/pages/tests/test_homepage.py::test_numqueries - Failed: Expected to perform 10 queries but 22 were done (add -v option to show queries)

🦺 DummyCache ou LocMemCache => tests OK 🍏

🦺 avec les settings de config.settings.base + COMPRESS_OFFLINE=False => tests KO 🧧

🦄 solution

  • forcer la compression avant l'execution des tests pour eviter les écritures en DB surnuméraires
  • harmoniser les settings utilisés par pytest en local et dans la CI

Type de changement

🪲 Correction de bug (changement non cassant qui corrige un problème).
🚧 technique

@vincentporte vincentporte added hot fix Pull requests that fix a bug to fix as soon as possible technical debt labels Dec 17, 2024
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

À mon avis, il serait préférable d’utiliser le même backend en test/dev et en prod pour réduire les différences entre les environnements, qui sont toujours une source de bugs.

@francoisfreitag
Copy link
Contributor

Je vois de nouvelles requêtes de la forme suivante :

SELECT "cache_key", "expires" FROM "cache_table" WHERE "cache_key" = ':1:django_compressor.mtime.b164c81e73ddd68c1f85f9d93dab28ccebbd52d6244d1f640b9ff96de5c6eed6'
INSERT INTO "cache_table" ("cache_key", "value", "expires") VALUES (':1:django_compressor.mtime.b164c81e73ddd68c1f85f9d93dab28ccebbd52d6244d1f640b9ff96de5c6eed6', 'gAWVCgAAAAAAAABHQdnYaLSiapQu',
 '2024-12-17T16:28:05+00:00'::timestamptz)

Je pense que les requêtes supplémentaires sont correctement capturées.

Je vois que la CI lance compress. Comme les requêtes supplémentaires viennent de django-compressor, je pense qu’il y a un bon suspect.

@vincentporte
Copy link
Contributor Author

vincentporte commented Dec 17, 2024

Je vois de nouvelles requêtes de la forme suivante :

SELECT "cache_key", "expires" FROM "cache_table" WHERE "cache_key" = ':1:django_compressor.mtime.b164c81e73ddd68c1f85f9d93dab28ccebbd52d6244d1f640b9ff96de5c6eed6'
INSERT INTO "cache_table" ("cache_key", "value", "expires") VALUES (':1:django_compressor.mtime.b164c81e73ddd68c1f85f9d93dab28ccebbd52d6244d1f640b9ff96de5c6eed6', 'gAWVCgAAAAAAAABHQdnYaLSiapQu',
 '2024-12-17T16:28:05+00:00'::timestamptz)

Je pense que les requêtes supplémentaires sont correctement capturées.

Je vois que la CI lance compress. Comme les requêtes supplémentaires viennent de django-compressor, je pense qu’il y a un bon suspect.

Je comprends parfaitement ton argument @francoisfreitag concernant le fait d'avoir des environnements les plus similaires possibles (d'autant que tu m'en as déjà parlé ;) )

L'autre option est une fixture limitée à la fonction, qui force la compression à chaque test.

import pytest
from django.core.management import call_command

@pytest.fixture(scope='function', autouse=True)
def run_compress(db):
    call_command('compress', '--force')

Les tests redeviennent passant, bien que sensiblement ralentis.

Qu'en penses-tu ?

@francoisfreitag
Copy link
Contributor

J’ai poussé un commit qui devrait te plaire :)

@vincentporte
Copy link
Contributor Author

J’ai poussé un commit qui devrait te plaire :)

je te confirme que ça me plait beaucoup ! en local, suite complète de tests en 1 min 54 contre 3min 15 quand la fixture a un scope function

@vincentporte vincentporte merged commit 66ca0f5 into master Dec 18, 2024
9 checks passed
@vincentporte vincentporte deleted the 853_cache_in_dev_env branch December 18, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hot fix Pull requests that fix a bug to fix as soon as possible technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants