Réinitialise le cache à chaque test de l'api #2761

DevHugo · 2015-05-29T14:33:40Z

Q	R
Correction de bugs ?	oui
Nouvelle Fonctionnalité ?	non
Tickets (issues) concernés	#2727

QA: Normalement travis suffit + relecture du code.

Si vous voulez tester dans votre environnement

Installer et activer Memcached
Lancer les tests, vérifier que ça marche pas
Passer sur la branche, lancer les tests, vérifier que ça marche

coveralls · 2015-05-29T14:52:26Z

Coverage increased (+0.01%) to 83.82% when pulling 3105611 on DevHugo:clean_cache into 2e3217a on zestedesavoir:dev.

landscape-bot · 2015-05-29T15:06:22Z

Code quality remained the same when pulling 3105611 on DevHugo:clean_cache into 2e3217a on zestedesavoir:dev.

GerardPaligot · 2015-05-29T16:14:08Z

Pratique qui a besoin d'être validé auprès de notre DTC (ping @SpaceFox).

Par hasard, est-ce que tu pourrais rajouter un test qui fait une requête sur la page 1 puis sur la page 2 et vérifier qu'il charge bien la page 1 pour la première requête et la page 2 pour la seconde ?

DevHugo · 2015-05-30T10:57:52Z

Pratique qui a besoin d'être validé auprès de notre DTC (ping @DevHugo).

C'est pas plutôt SpaceFox que tu voulais ping ?

Par hasard, est-ce que tu pourrais rajouter un test qui fait une requête sur la page 1 puis sur la
page 2 et vérifier qu'il charge bien la page 1 pour la première requête et la page 2 pour la seconde ?

Ok, je fais ça dans la semaine

landscape-bot · 2015-05-30T15:42:45Z

Code quality remained the same when pulling c5ee84e on DevHugo:clean_cache into 2e3217a on zestedesavoir:dev.

GerardPaligot · 2015-05-30T15:47:03Z

Bien, si Travis passe sur ces nouveaux assertions, la cause du bug de cache sur l'API en prod sera sans doute de l'infrastructure. Merci @DevHugo pour le test.

DevHugo · 2015-05-30T15:55:45Z

Je suis heureux, que le test plante au moins, c'est pas que en prod ! Je l'ai pas sur mon env de dev à moi mais c'est cool ! ^^

DevHugo · 2015-05-30T17:12:21Z

Il ont discuté du bug dans cette conversation IRC (Pour une fois que l'irc sert à quelque chose =) ) .

Les logs sont ici, mais il faut y aller avec la fonction rechercher, le mots clés est 'Pagination' et l'auteur est gmorell: https://botbot.me/freenode/restframework/2015-05-23/?tz=Europe%2FLondon&page=1.

TLDR: Bug upstream dans ce fichier.

La solution pour résoudre le problème rapidement est donné par gmorell. Il a créé une classe DJRF3xPaginationKeyBit qui corrige la classe PaginationKeyBit.

Dans notre code, on aurait,

class PagingPrivateTopicListKeyConstructor(DefaultKeyConstructor):
    pagination = DJRF3xPaginationKeyBit()

Faut se taper toute les classes qui utilise de la Pagination, et en plus faudrait re-changer quand la solution arrivera dans l'upstream. Mais elle marche, j'ai testé.

Deux solutions:

On attend qu'il corrige ça upstream (pas de PR en cours sur le sujet).
Je créé une PR avec la correction de Gmorell et faudrat reverse ma PR quand la solution arrivera.

Mais bon, pour moi, on est un peu hors-sujet. Les tests ne passe pas à cause du cache pas réinitialisé et pas de cette erreur qui est une autre issue pour moi.

landscape-bot · 2015-05-30T17:57:53Z

Repository health decreased by 0.01% when pulling 68618a6 on DevHugo:clean_cache into 2e3217a on zestedesavoir:dev.

1 new problem was found (including 1 error and 0 code smells).
No problems were fixed.

DevHugo · 2015-05-30T17:59:34Z

Même si travis passe, NE SURTOUT PAS MERGE !

Le prochain commit applique la correction de gmorell.

Edit: la correction marche plutôt bien.

landscape-bot · 2015-05-30T18:06:39Z

Code quality remained the same when pulling 179e574 on DevHugo:clean_cache into 1e1c05e on zestedesavoir:dev.

landscape-bot · 2015-05-30T18:25:38Z

Code quality remained the same when pulling 226594b on DevHugo:clean_cache into 1e1c05e on zestedesavoir:dev.

Eskimon · 2015-06-02T12:11:46Z

Du coup "quoi de neuf ici" ?

GerardPaligot · 2015-06-02T12:18:07Z

Il faut squasher les 2 derniers commits
Avoir une QA sur l'API vers des requêtes paginées et un cache (iso prod si possible)
Avoir des avis sur la pratique qui réinitilise le cache.

DevHugo · 2015-06-03T10:53:12Z

Avant de squash, les deux commits, j'aimerais bien qu'on confirme qu'on sépare pas la résolution en deux PR distincte: une pr pour corriger les tests unitaires et une PR pour le problème de cache de la pagination.

Après faut faire un choix entre ses deux solutions:

On attend qu'il corrige ça upstream (pas de PR en cours sur le sujet).
Je créé une PR avec la correction de Gmorell et faudrat reverse ma PR quand la solution arrivera.

Ensuite comme GerardPaligot dit :

Avoir une QA sur l'API vers des requêtes paginées et un cache (iso prod si possible)
Avoir des avis sur la pratique qui réinitilise le cache.

GerardPaligot · 2015-06-04T11:15:55Z

Inutile de dupliquer les tags "Bloquant". L'issue l'est déjà.

Eskimon · 2015-06-16T09:03:50Z

N'etant pas vraiment connaisseur du code, perso je te fais assez confiance sur ca @GerardPaligot . Si tu penses que le code est clean alors moi ca me va.

Ensuite pour la QA, je sais que c'est pas terrible ce que je vais proposer mais on a pas vraiment le choix, ce souci est bloquant et il faut avancer, donc voila ce que je propose...

La preprod est actuellement sur l'ancienne release avec une API qui remarche
Je me met dessus, fais une branche temporaire et cherry-pick le dernier commit de DevHugo
Vous testez ce qu'il faut
Je reviens sur la branche courante en preprod et vire la temporaire.

Ca vous va ?

EDIT : Je rappel que cette PR est celle qui potentiellement resout notre dernier souci bloquant pour une release qui amenera entre autres la nouvelle home page...

GerardPaligot · 2015-06-16T09:16:44Z

N'etant pas vraiment connaisseur du code, perso je te fais assez confiance sur ca @GerardPaligot . Si tu penses que le code est clean alors moi ca me va.

Pour moi, c'est le meilleur compromis. Je voulais l'avis de notre DTC (@SpaceFox) mais je n'ai jamais pu l'avoir.

Ensuite pour la QA, je sais que c'est pas terrible ce que je vais proposer mais on a pas vraiment le choix, ce souci est bloquant et il faut avancer, donc voila ce que je propose...

Pourquoi ne pas tirer la branche sur la bêta ?

Eskimon · 2015-06-16T09:19:21Z

parce que la branche est synchro avec dev, tu coup ca va ramener plein de bordel et faudra que je fasse l'equivelent d'une mise en prod' pour faire les choses bien et ca m'ennuie un peu (car besoin de mettre a jour les requirements etc... bref, je le sens moyen :D alors que le cherry-pick me permettera de tester de maniere vraiment atomique juste ce commit)

J'en profite pour ping @pierre-24 qui fait une tentative chez lui en ce moment meme :)

DevHugo · 2015-06-16T09:20:24Z

Il faut être sur que le cache est configuré correctement sur la préprod, je squash les deux derniers commits dans quelques minutes.

Je peux rebase sur une branche particulière, si tu veux.

Eskimon · 2015-06-16T09:21:03Z

Je peux rebase sur une branche particulière, si tu veux.

C'est pas necessaire non :)

Il faut être sur que le cache est configuré correctement sur la préprod, je squash les deux derniers commits dans quelques minutes.

Normalement oui car on est cense etre iso-prod

DevHugo · 2015-06-16T09:26:18Z

Faut créé une issue pour permettre de penser à reverse le commit quand la modification apparaîtra dans le master de drf-extensions.

landscape-bot · 2015-06-16T09:26:42Z

Repository health increased by 0.01% when pulling 497f91b on DevHugo:clean_cache into 4438ed0 on zestedesavoir:dev.

No new problems were introduced.
1 problem was fixed (including 0 errors and 0 code smells).

pierre-24 · 2015-06-16T09:30:34Z

Bon, vous allez un peux vite pour que je suive, mais QA ok pour moi. Juste, qu'est ce que "bug de pagination" veux dire ?

DevHugo · 2015-06-16T09:42:29Z

Pour constater le bug sur la prod:

Tu prend n'importe qu'elle méthode qui demande la pagination. Exemple: Pour lister les membres, c'est l'adresse: https://zestedesavoir.com/api/membres/?page=1
Tu demande maintenant la page 3, par exemple, https://zestedesavoir.com/api/membres/?page=2. Tu tombe sur les résultats de la page 6.

Ce qu'ils faut tester aussi, tu active le cache (Memcached), et tu passe les tests unitaires.

pierre-24 · 2015-06-16T11:29:04Z

Ok pour la pagination. Par contre, je me tape une erreur qui a téoriquement rien à voir:

======================================================================
FAIL: test_create_topic (zds.forum.tests.tests.ForumMemberTests)
To test all aspects of topic's creation by member.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pbeaujea/Devels/zds-site/zds/forum/tests/tests.py", line 137, in test_create_topic
    self.assertContains(response, self.forum11.title)
  File "/home/pbeaujea/Devels/zds-site/zdsenv/local/lib/python2.7/site-packages/django/test/testcases.py", line 363, in assertContains
    msg_prefix + "Couldn't find %s in response" % text_repr)
AssertionError: Couldn't find 'Mon Forum No239' in response

DevHugo · 2015-06-16T11:40:10Z

J'ai corrigé des tests donc c'est possible que ça soit liés. Tu avais memcache d'activé ? tu as lancé les tests avec quels commandes ? tu as l'ordre dans lequels se sont effectué ?

Eskimon · 2015-06-16T11:40:40Z

Bah c'est bizarre ce test ne rentre pas dans la liste des tests affecte par tes changements...

DevHugo · 2015-06-16T11:47:01Z

Je confirme, j'ai la même erreur en local.

Memcached d'activé, python manage.py test

pierre-24 · 2015-06-16T11:48:39Z

Je l'ai sur dev aussi, mais j'imagine que c'est normal ... Je devrais essayer en désactivant memcached ?

DevHugo · 2015-06-16T11:51:44Z

En désactivant memcache, ça marche =(.

pierre-24 · 2015-06-16T11:52:09Z

Donc c'est lié :(

DevHugo · 2015-06-16T11:52:50Z

J'ai pas le temps aujourd'hui, qui veut regarde.

DevHugo · 2015-06-16T11:53:34Z

Pourquoi travis ne bug pas alors ?

Eskimon · 2015-06-16T11:53:45Z

et en ramenant ca ? #2756

DevHugo · 2015-06-16T12:01:50Z

C'est peut-être ça mais ça explique pas pourquoi travis ne bug pas.

Pas le temps de testé, si vous voulez vous pouvez refaire une PR en cherry pick mes deux commits pour aller plus vite car pas très dispo en ce moment.

(zdsenvpython2)~/d/p/zds-site git:dev ❯❯❯ python manage.py test zds.forum                       ⏎ ✭ ✱ ◼
Creating test database for alias 'default'...
..................Mon Forum No91
F.......................................................................................................................................................
======================================================================
FAIL: test_create_topic (zds.forum.tests.tests.ForumMemberTests)
To test all aspects of topic's creation by member.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/hugo/dev/python/zds-site/zds/forum/tests/tests.py", line 139, in test_create_topic
    self.assertContains(response, self.forum11.title)
  File "/home/hugo/dev/python/zdsenvpython2/lib/python2.7/site-packages/django/test/testcases.py", line 363, in assertContains
    msg_prefix + "Couldn't find %s in response" % text_repr)
AssertionError: Couldn't find 'Mon Forum No91' in response

----------------------------------------------------------------------
Ran 170 tests in 22.644s

FAILED (failures=1)
Destroying test database for alias 'default'...
(zdsenvpython2)~/d/p/zds-site git:dev ❯❯❯ systemctl status memcached                            ⏎ ✭ ✱ ◼
● memcached.service - Memcached Daemon
   Loaded: loaded (/usr/lib/systemd/system/memcached.service; disabled; vendor preset: disabled)
   Active: active (running) since mar. 2015-06-16 13:53:17 CEST; 21min ago
 Main PID: 10460 (memcached)
   CGroup: /system.slice/memcached.service
           └─10460 /usr/bin/memcached -l 127.0.0.1
(zdsenvpython2)~/d/p/zds-site git:dev ❯❯❯ systemctl stop memcached                                ✭ ✱ ◼
(zdsenvpython2)~/d/p/zds-site git:dev ❯❯❯ systemctl status memcached                              ✭ ✱ ◼
● memcached.service - Memcached Daemon
   Loaded: loaded (/usr/lib/systemd/system/memcached.service; disabled; vendor preset: disabled)
   Active: inactive (dead)
(zdsenvpython2)~/d/p/zds-site git:dev ❯❯❯ python manage.py test zds.forum                       ⏎ ✭ ✱ ◼
Creating test database for alias 'default'...
..................Mon Forum No91
........................................................................................................................................................
----------------------------------------------------------------------
Ran 170 tests in 23.496s

OK
Destroying test database for alias 'default'...
(zdsenvpython2)~/d/p/zds-site git:dev ❯❯❯

DevHugo · 2015-06-17T10:53:40Z

Avec la PR de spacefox, les tests repassent. Good to go !

Eskimon · 2015-06-17T10:56:58Z

Avec la PR de spacefox, les tests repassent. Good to go !

Avant de merger il faudra donc "juste" verifier que la PR de spacefox ne casse rien, puis on la merge, puis celle-ci, puis on release, puis on conquit le monde, puis on se repose. Ok ?

DevHugo · 2015-06-17T11:01:30Z

Ça me semble être un bon bon programme !

2015-06-17 12:56 GMT+02:00 Eskimon notifications@github.com:

Avec la PR de spacefox, les tests repassent. Good to go !

Avant de merger il faudra donc "juste" verifier que la PR de spacefox ne
casse rien, puis on la merge, puis celle-ci, puis on release, puis on
conquit le monde, puis on se repose. Ok ?

—
Reply to this email directly or view it on GitHub
#2761 (comment)
.

Eskimon · 2015-06-17T12:33:20Z

J'ai repris ici : #2816 en rebasant pour intégrer les modifications apportées par spacefox.

DevHugo changed the title ~~Réinitialise le cache à chaque test~~ Réinitialise le cache à chaque test de l'api May 29, 2015

Situphen added S-BUG Corrige un problème Bloquant Ticket qui doit être traité avant la prochaine mise à jour C-API Concerne une API du site labels May 31, 2015

GerardPaligot mentioned this pull request Jun 2, 2015

Lien de l'API #2596

Closed

GerardPaligot removed the Bloquant Ticket qui doit être traité avant la prochaine mise à jour label Jun 4, 2015

DevHugo added 2 commits June 16, 2015 11:22

Réinitialise le cache à chaque test

fc05424

Corrige le bug de pagination

497f91b

Eskimon closed this Jun 17, 2015

Eskimon mentioned this pull request Jun 17, 2015

Réinitialise le cache à chaque test de l'api #2816

Merged

DevHugo mentioned this pull request Jun 17, 2015

[Reminder] Reverse le code de la pagination #2817

Closed

Réinitialise le cache à chaque test de l'api #2761

Réinitialise le cache à chaque test de l'api #2761

Conversation

DevHugo commented May 29, 2015

coveralls commented May 29, 2015

landscape-bot commented May 29, 2015

GerardPaligot commented May 29, 2015

DevHugo commented May 30, 2015

landscape-bot commented May 30, 2015

GerardPaligot commented May 30, 2015

DevHugo commented May 30, 2015

DevHugo commented May 30, 2015

landscape-bot commented May 30, 2015

DevHugo commented May 30, 2015

landscape-bot commented May 30, 2015

landscape-bot commented May 30, 2015

Eskimon commented Jun 2, 2015

GerardPaligot commented Jun 2, 2015

DevHugo commented Jun 3, 2015

GerardPaligot commented Jun 4, 2015

Eskimon commented Jun 16, 2015

GerardPaligot commented Jun 16, 2015

Eskimon commented Jun 16, 2015

DevHugo commented Jun 16, 2015

Eskimon commented Jun 16, 2015

DevHugo commented Jun 16, 2015

landscape-bot commented Jun 16, 2015

pierre-24 commented Jun 16, 2015

DevHugo commented Jun 16, 2015

pierre-24 commented Jun 16, 2015

DevHugo commented Jun 16, 2015

Eskimon commented Jun 16, 2015

DevHugo commented Jun 16, 2015

pierre-24 commented Jun 16, 2015

DevHugo commented Jun 16, 2015

pierre-24 commented Jun 16, 2015

DevHugo commented Jun 16, 2015

DevHugo commented Jun 16, 2015

Eskimon commented Jun 16, 2015

DevHugo commented Jun 16, 2015

DevHugo commented Jun 17, 2015

Eskimon commented Jun 17, 2015

DevHugo commented Jun 17, 2015

Eskimon commented Jun 17, 2015