-
Notifications
You must be signed in to change notification settings - Fork 161
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
[hotfix] stopper les erreurs unicodes générées par la validation #2585
Conversation
|
C'est bien un hotfix. Je corrige le titre. |
Un lien avec #2588 ? |
Comme a dit Firm1 de l'autre côté: mon fix empêche la validation de planter quand un bug surgit dans le markdown. Son fix corrige l'erreur qui est à la base de nos soucis actuels dans le markdown. (Après, @firm1, on aurait probablement pu faire deux en un, non ?) |
En effet. Mais j'avoue que je n'ai pas vu ta PR au moment ou j'ai fais la Le mer. 22 avr. 2015 12:52, Pierre Beaujean notifications@github.com a
|
... Et à réflexion, mon fix va être dûr à tester si le markdown ne plante plus (ce qui est très rare, à priori, je ne suis jamais parvenu à le faire planter autrement) |
En même temps si le markdown plante le contenu ne s'affiche plus. C'est donc très critique et je fix ça le plus rapidement possible (c'est presque les seules choses que je maintient sur le markdown). De plus ton cas est en fait rare. C'est arrivé d'autres fois mais en général détecté avant sur le forum. |
C'est mergeable ici ? |
personne y veut faire ma QA :'( (sinon, oui, mais les règles sont les règles) |
Personne pour QA ici ? Ne m'obligez pas à sortir le chat de Shrek :( |
Je m'en charge ce soir (sauf si quelqu'un l'a fait avant). |
Si je me trompe pas, il suffit de tester avec les tutos (mini et big) des fixtures de test et ça devrait l'faire (en tout cas chez moi ça plantait avant...) |
Rapport de QA : NOK A la validation du tutoriel, j'obtiens l'erreur suivante : Internal Server Error: /tutoriels/validation/valid/
Traceback (most recent call last):
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/core/handlers/base.py", line 112, in get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/member/decorator.py", line 19, in _can_write_and_read_now
return func(request, *args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/auth/decorators.py", line 22, in _wrapped_view
return view_func(request, *args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/views/decorators/http.py", line 41, in inner
return func(request, *args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/auth/decorators.py", line 22, in _wrapped_view
return view_func(request, *args, **kwargs)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/tutorial/views.py", line 351, in valid_tutorial
err = mep(tutorial, tutorial.sha_validation)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/tutorial/views.py", line 3216, in mep
get_url_images(md_file_contenu, prod_path)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/tutorial/views.py", line 3091, in get_url_images
urlretrieve(real_url, down_path)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urllib.py", line 94, in urlretrieve
return _urlopener.retrieve(url, filename, reporthook, data)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urllib.py", line 228, in retrieve
url = unwrap(toBytes(url))
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urllib.py", line 1055, in toBytes
" contains non-ASCII characters")
UnicodeError: URL u'http://fr.wikipedia.org/wiki/Cl\xe9mentine' contains non-ASCII characters
[27/Apr/2015 23:56:10] "POST /tutoriels/validation/valid/ HTTP/1.1" 500 136762 |
try: | ||
html_file.write(emarkdown(md_file_contenu, is_js)) | ||
except UnicodeEncodeError: | ||
return _(u'Une erreur est survenue lors de la génération du HTML, vérifiez que le code markdown ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Faire un return
uniquement si tu rencontres une erreur, c'est crade. Lance une exception ou intercepte l'exception UnicodeEncodeError
dans les méthodes appelantes mais ne fait pas de return
uniquement quand tu rencontres une exception ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Celle là, c'est originalement de firm1 ^^
Mais ok, je vais faire ça.
fbf2a75
to
e24bc9a
Compare
Et voilà, ça lance l'erreur qui va bien :) |
|
Avant que vous ne dites que Travis me râle dessus, je vous prévient tout de suite, c'est le front qui m'as laché, pour dépassement du temps limite. À qui de droit de relancer Travis demain, aujourd'hui, ça fait 2x que je relance Travis et ça va pas mieux ;) |
Je refais une QA ce soir. En tout cas, le code est nettement plus clean. Good job! |
Bien, j'ai fais la QA. J'ai bien vérifié que j'avais ma branche à jour parce que j'ai toujours une erreur python à la place d'être stoppé par le back. Est-ce qu'il faut faire quelque chose de spécial ? Internal Server Error: /tutoriels/validation/valid/
Traceback (most recent call last):
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/core/handlers/base.py", line 112, in get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/member/decorator.py", line 19, in _can_write_and_read_now
return func(request, *args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/auth/decorators.py", line 22, in _wrapped_view
return view_func(request, *args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/views/decorators/http.py", line 41, in inner
return func(request, *args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/auth/decorators.py", line 22, in _wrapped_view
return view_func(request, *args, **kwargs)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/tutorial/views.py", line 352, in valid_tutorial
mep(tutorial, tutorial.sha_validation)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/tutorial/views.py", line 3222, in mep
get_url_images(md_file_contenu, prod_path)
File "/Users/Gerard/Documents/workspace-django/zds-site/zds/tutorial/views.py", line 3091, in get_url_images
urlretrieve(real_url, down_path)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urllib.py", line 94, in urlretrieve
return _urlopener.retrieve(url, filename, reporthook, data)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urllib.py", line 228, in retrieve
url = unwrap(toBytes(url))
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urllib.py", line 1055, in toBytes
" contains non-ASCII characters")
UnicodeError: URL u'http://fr.wikipedia.org/wiki/Cl\xe9mentine' contains non-ASCII characters
[28/Apr/2015 20:50:34] "POST /tutoriels/validation/valid/ HTTP/1.1" 500 136867 |
Oh, j'ai compris ce qui n'allais pas. Je catche pas les erreurs de cette fonction là |
Placer un try/catch sur toute une méthode parce qu'il peut y avoir une erreur quelque part, c'est pas un peu mettre un try/catch Exception dans une méthode main en Java et se dire que de toute façon, on interceptera tous les bugs à ce niveau là pour pas faire crasher l'application ? Quelles sont les instructions précises qui peuvent lancer une exception ? |
Ben à la base, je pensais que c'était seulement |
C'est bizarre parce que j'attache pas d'image à mon tutoriel dans ma QA. |
Mon code de QA ( |
Et on sait pourquoi ça fait crasher un tutoriel mais pas un article ? |
Bien sur: le code, implémenté par Firm1, tente de récupérer les images d'un tuto et de les héberger chez nous dans le processus de validation. Mais ce mécanisme n'as été implémenté que dans les tutos. |
(par contre, l'erreur UTF-8 quand markdown nous lache, c'est aussi bien sur les articles que sur les tutos, et ça, j'ai bien fait ce qui fallait coté article aussi) |
Ok et ce
Comment il balance des noms sans les pinger ! :-° |
My bad: implémenté par @firm1
Si, et c'est ce que je vais essayer ;) |
Sorry mais quand je fais de la QA, je ne le fais pas que fonctionnellement. Je peux être un peu chiant. |
Que du contraire, c'est le bon comportement :) |
97a8c25
to
353d68c
Compare
Hop, voilà, c'est corrigé. C'est complètement ridicule, parce que je fait un NOTE TRÈS IMPORTANTE: testez plutôt avec
Sinon, c'est PIL qui se met à râler parce qu'il connait pas l’extension (encore une erreur pas catchée, soit dit au passage !!!!). Et moi, toute cette histoire d'encodage commence tout doucement à me donner envie que le passage à Python 3 soit effectif :o |
|
@@ -3233,7 +3242,12 @@ def mep(tutorial, sha): | |||
else: | |||
is_js = "" | |||
if md_file_contenu is not None: | |||
html_file.write(emarkdown(md_file_contenu, is_js)) | |||
try: | |||
html_file.write(emarkdown(md_file_contenu, is_js)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pas moyen d'encoder en UTF-8 aussi pour éviter les erreurs ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non, et c'est là que ça devient fortement ridicule. emarkdown()
renvois une en ascii MAIS si erreur il y a, l'erreur est en utf-8. Bref, une fois sur deux, le decode()
pose plus de problème qu'il n'en résous. Conclusion: le problème est beaucoup plus profond que ce que ce patch innocent ne le prétend, et une bonne partie de tout ça est à revoir, ou alors il faut espérer que le passage à python 3 harmonisera un peu les choses (tout sera alors en utf-8). Bonus: get_blob()
, la fonction qui va piocher dans le dépôt Git, renvois du ... byte
.
Tu m'étonnes qu'on a autant de m*** sur la validation !!
(je vais essayer de faire au mieux dans la ZEP-12, mais ici, c'est clairement peine perdue et on va avoir des codes monstrueusement horribles, et j'en suis le premier désolé).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, la raison me semble tout à fait cohérente.
Je test la PR ce soir ! |
Alors, je ne sais pas si ça vient de chez moi mais j'obtiens l'erreur suivante : Internal Server Error: /tutoriels/off/1/ez/
Traceback (most recent call last):
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/core/handlers/base.py", line 199, in get_response
response = middleware_method(request, response)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/debug_toolbar/middleware.py", line 89, in process_response
new_response = panel.process_response(request, response)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/debug_toolbar/panels/request.py", line 55, in process_response
for k in sorted(request.session.keys(), key=force_text)]
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/sessions/backends/base.py", line 118, in keys
return self._session.keys()
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/sessions/backends/base.py", line 173, in _get_session
self._session_cache = self.load()
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/contrib/sessions/backends/cached_db.py", line 42, in load
expire_date__gt=timezone.now()
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/manager.py", line 151, in get
return self.get_queryset().get(*args, **kwargs)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/query.py", line 304, in get
num = len(clone)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/query.py", line 77, in __len__
self._fetch_all()
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/query.py", line 857, in _fetch_all
self._result_cache = list(self.iterator())
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/query.py", line 220, in iterator
for row in compiler.results_iter():
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 713, in results_iter
for rows in self.execute_sql(MULTI):
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 786, in execute_sql
cursor.execute(sql, params)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/debug_toolbar/panels/sql/tracking.py", line 159, in execute
return self._record(self.cursor.execute, sql, params)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/debug_toolbar/panels/sql/tracking.py", line 101, in _record
return method(sql, params)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/backends/util.py", line 69, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/backends/util.py", line 53, in execute
return self.cursor.execute(sql, params)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/utils.py", line 99, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/backends/util.py", line 53, in execute
return self.cursor.execute(sql, params)
File "/Users/Gerard/.virtualenvs/zdsenv/lib/python2.7/site-packages/django/db/backends/sqlite3/base.py", line 452, in execute
return Database.Cursor.execute(self, query, params)
OperationalError: no such table: django_session Au départ, je me suis dis que ça devait venir de ma base de données mais je l'ai supprimé puis je l'ai re-généré mais l'erreur était toujours là. Donc voilà, c'est bizarre. |
C'est reproductible. Ca survient que quand je tente de faire ta QA. Par exemple, si je fais une MEP d'un tuto valide, je n'ai pas de soucis. |
Alors, j'ai deux chose à dire à ça: tu as le bon comportement (enfin), et non je ne sais pas d'ou ça vient. Ma première remarque est basée sur le fait que si tu redémarre Django, tu devrais voir le message que je tente de te faire faire apparaitre depuis le début, et donc cette PR rempli(rai?) son but. Quand à ma deuxième remarque, j'en suis absolument conscient (je le sais depuis que j'ai commencé la PR) et j'ai mis ça sur le compte du changement Django 1.6-<->1.7. J'en veux pour preuve ceci : http://stackoverflow.com/a/5869933
Pour moi, le problème vient de ces petits messages verts/oranges. Évidement, si tu me dis que lesdits messages apparaissent bien normalement, je suis completement perdu. |
Malheureusement, et puisque c'est reproductible sur des environnements différents, je ne peux pas dire que la QA est ok (même si je vois bien le message après redémarrage de Django). En prod, on pourra pas se permettre de redémarrer Django à chaque MEP foireuse. |
Ah, mais je dis pas ça pour forcer quoique ce soit. Mais voyons les choses de manière objective: what the fuck ? À aucun moment dans la backtrace, le code ne va chercher quelque chose chez nous, c'est pûrement chez Django que ça se passe. Pire que ça, je sais pas ce qu'il cherche (sachant que notre ... Bref. Je sèche, sur celle là. Et je sature, aussi, parce que tout ça n'as aucun sens et que tout le module des tutos ET python jouent contre moi. @SpaceFox : fait la MEP en prod sans cette PR, je fais traîner tout le bordel et j'arrive à rien. Tant pis, le markdown à qu'à pas planter, c'est tout. Puis les gens voient bien que ça avance pas, et comme ce tuto est important pour le site ... |
(de toute façon, #2588 règle le problème. De la mauvaise façon, mais le problème est réglé, donc on est bon) |
J'ai fais toute cette QA pour rien ? T.T |
Bah oui, désolé, mais tu vois bien que j'arrive à rien. |
Bon. J'étais de très très très très (très)¹⁰⁰⁰⁰⁰ mauvaise humeur tantôt, désolé (rien à voir avec ZdS, d'ailleurs). Il se trouve que malgré tout mes essais, le bug avec django_session tient du bug mystique. J'ai donc décidé d'aller fixer ça en v1.8 (d'autant que comme je le dis, #2588 règle le problème à la source, donc pas de la bonne manière, mais ça marchera). |
(je ne sais pas si c'est un vrai bugfix à faire en prod' ou si c'est un bêtafix, donc corrigez moi si nécessaire).
Note de QA
DOWNGRADEZ VOTRE VERSION DE MARKDOWN SUR LA VERSION ACTUELEMENT EN PROD OU VOUS NE SAUREZ PAS REPRODUIRE LE PROBLEME
![cette image va planter](http://fr.wikipedia.org/wiki/Clémentine.png) et [ce lien va rater](http://fr.wikipedia.org/wiki/Clémentine)
(paradoxal, non ?). Vérifiez ensuite que markdown n'est pas contentNote: puisque ce bug exploite un bug dans le markdown qui devrait disparaitre à un moment ou un autre, il va être quasiment impossible d'écrire un T.U., sauf si quelqu'un a une idée.