Merge lp:~elachuni/ubuntu-webcatalog/amqp-oopses into lp:ubuntu-webcatalog
- amqp-oopses
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 178 |
Proposed branch: | lp:~elachuni/ubuntu-webcatalog/amqp-oopses |
Merge into: | lp:ubuntu-webcatalog |
Diff against target: |
338 lines (+153/-24) 11 files modified
.bzrignore (+1/-0) django_project/config/main.cfg (+2/-12) django_project/urls.py (+4/-0) setup.py (+1/-1) src/webcatalog/management/commands/runserver.py (+36/-0) src/webcatalog/schema.py (+2/-0) src/webcatalog/templates/500.html (+3/-2) src/webcatalog/tests/test_oops_wsgi_config.py (+12/-4) src/webcatalog/urls.py (+1/-0) src/webcatalog/views.py (+58/-2) src/webcatalog/wsgi.py (+33/-3) |
To merge this branch: | bzr merge lp:~elachuni/ubuntu-webcatalog/amqp-oopses |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Łukasz Czyżykowski (community) | Approve | ||
Review via email: mp+151051@code.launchpad.net |
Commit message
Display the oops id on every error page, and generate oops reports in dev.
Description of the change
This branch includes several improvements related to ampq oops publishing:
- A custom wsgi handler was added that generates an oops id for every exception.
- Switched to oops-dictconfig 0.0.5 to use the 'inherit_id' and 'new_only' settings.
- A custom runserver command was added that uses webcatalog's wsgi stack in dev (to generate oopses in dev)
Łukasz Czyżykowski (lukasz-czyzykowski) wrote : | # |
Łukasz Czyżykowski (lukasz-czyzykowski) : | # |
Ubuntu One Auto Pilot (otto-pilot) wrote : | # |
The attempt to merge lp:~elachuni/ubuntu-webcatalog/amqp-oopses into lp:ubuntu-webcatalog failed. Below is the output from the failed tests.
[localhost] local: /usr/bin/python /usr/bin/virtualenv --no-site-packages virtualenv
The --no-site-packages flag is deprecated; it is now the default behavior.
New python executable in virtualenv/
Installing distribute.
Installing pip....
[localhost] local: virtualenv/bin/pip install -r test_requiremen
Downloading/
Running setup.py egg_info for package django
Downloading/
Running setup.py egg_info for package coverage
warning: no previously-included files matching '*.pyc' found anywhere in distribution
Downloading/
Running setup.py egg_info for package mock
warning: no files found matching '*.png' under directory 'docs'
warning: no files found matching '*.css' under directory 'docs'
warning: no files found matching '*.html' under directory 'docs'
warning: no files found matching '*.js' under directory 'docs'
Downloading/
Downloading piston-
Running setup.py egg_info for package piston-mini-client
Downloading/
Running setup.py egg_info for package oauthlib
Downloading/
Running setup.py egg_info for package httplib2
Installing collected packages: django, coverage, mock, piston-mini-client, oauthlib, httplib2
Running setup.py install for django
changing mode of build/scripts-
changing mode of /mnt/tarmac/
Running setup.py install for coverage
building 'coverage.tracer' extension
gcc -pthread -fno-strict-
gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-
warning: no previously-included files matching '*.pyc' found anywhere in distribution
Installing coverage2 script to /mnt/tarmac/
Installing coverage-2.7 script to /mnt/tarmac/
Installing coverage script to /mnt/tarmac/
Running setup.py install for mock
warning: no files found matching '*.png' under directory 'docs'
warning: no files found matching '*.css' under directory ...
Preview Diff
1 | === modified file '.bzrignore' |
2 | --- .bzrignore 2012-04-18 09:37:51 +0000 |
3 | +++ .bzrignore 2013-03-01 12:52:22 +0000 |
4 | @@ -15,3 +15,4 @@ |
5 | django_project/adminaudit |
6 | django_project/local.cfg |
7 | django_project/sreclient.py |
8 | +oopses/ |
9 | |
10 | === modified file 'django_project/config/main.cfg' |
11 | --- django_project/config/main.cfg 2012-08-23 15:12:54 +0000 |
12 | +++ django_project/config/main.cfg 2013-03-01 12:52:22 +0000 |
13 | @@ -11,14 +11,14 @@ |
14 | debug = true |
15 | media_root = django_project/media_root_dev/ |
16 | |
17 | -installed_apps = webcatalog |
18 | - django.contrib.contenttypes |
19 | +installed_apps = django.contrib.contenttypes |
20 | django.contrib.sessions |
21 | django.contrib.sites |
22 | django.contrib.messages |
23 | django.contrib.staticfiles |
24 | django.contrib.markup |
25 | django.contrib.admin |
26 | + webcatalog |
27 | django_openid_auth |
28 | django_configglue |
29 | django.contrib.auth |
30 | @@ -106,16 +106,6 @@ |
31 | canonical-losas = admin |
32 | canonical-ca-hackers = developers |
33 | |
34 | -[oops_wsgi] |
35 | -oopses = oops_publishers |
36 | - |
37 | -[oops_publishers] |
38 | -publishers = oops_datedir_publisher |
39 | - |
40 | -[oops_datedir_publisher] |
41 | -type = datedir |
42 | -error_dir = oopses |
43 | - |
44 | [webcatalog] |
45 | serve_site_media = True |
46 | sca_api_url = https://sc.staging.ubuntu.com/api/2.0/ |
47 | |
48 | === modified file 'django_project/urls.py' |
49 | --- django_project/urls.py 2012-06-21 20:18:44 +0000 |
50 | +++ django_project/urls.py 2013-03-01 12:52:22 +0000 |
51 | @@ -25,6 +25,10 @@ |
52 | admin.autodiscover() |
53 | preflight.autodiscover() |
54 | |
55 | +handler500 = 'webcatalog.views.server_error' |
56 | +handler404 = 'webcatalog.views.page_not_found' |
57 | + |
58 | + |
59 | urlpatterns = patterns('', |
60 | url(r'^cat/', include('webcatalog.urls')), |
61 | url(r'^admin/', include(admin.site.urls)), |
62 | |
63 | === modified file 'setup.py' |
64 | --- setup.py 2012-09-28 15:02:24 +0000 |
65 | +++ setup.py 2013-03-01 12:52:22 +0000 |
66 | @@ -53,7 +53,7 @@ |
67 | 'celery==2.5.0', |
68 | 'django-celery==2.5.0', |
69 | 'oops-wsgi==0.0.10', |
70 | - 'oops-dictconfig==0.0.2', |
71 | + 'oops-dictconfig==0.0.5', |
72 | 'oops-datedir_repo==0.0.17', |
73 | ], |
74 | package_data = find_packages_data('src'), |
75 | |
76 | === added file 'src/webcatalog/management/commands/runserver.py' |
77 | --- src/webcatalog/management/commands/runserver.py 1970-01-01 00:00:00 +0000 |
78 | +++ src/webcatalog/management/commands/runserver.py 2013-03-01 12:52:22 +0000 |
79 | @@ -0,0 +1,36 @@ |
80 | +# -*- coding: utf-8 -*- |
81 | +# This file is part of the Apps Directory |
82 | +# Copyright (C) 2011-2013 Canonical Ltd. |
83 | +# |
84 | +# This program is free software: you can redistribute it and/or modify |
85 | +# it under the terms of the GNU Affero General Public License as |
86 | +# published by the Free Software Foundation, either version 3 of the |
87 | +# License, or (at your option) any later version. |
88 | +# |
89 | +# This program is distributed in the hope that it will be useful, |
90 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
91 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
92 | +# GNU Affero General Public License for more details. |
93 | +# |
94 | +# You should have received a copy of the GNU Affero General Public License |
95 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
96 | + |
97 | +"""Customized runserver command that runs ubuntu-webcatalog's wsgi stack.""" |
98 | + |
99 | +from optparse import make_option |
100 | +from django.core.management.commands.runserver import BaseRunserverCommand |
101 | +from django.core.servers.basehttp import AdminMediaHandler |
102 | + |
103 | + |
104 | +class Command(BaseRunserverCommand): |
105 | + option_list = BaseRunserverCommand.option_list + ( |
106 | + make_option( |
107 | + '--adminmedia', dest='admin_media_path', default='', |
108 | + help='Specifies the directory from which to serve admin media.'), |
109 | + ) |
110 | + |
111 | + def get_handler(self, *args, **options): |
112 | + """Serves admin media like old-school (deprecation pending).""" |
113 | + from webcatalog.wsgi import make_app |
114 | + handler = make_app() |
115 | + return AdminMediaHandler(handler, options.get('admin_media_path', '')) |
116 | |
117 | === modified file 'src/webcatalog/schema.py' |
118 | --- src/webcatalog/schema.py 2013-02-22 21:02:05 +0000 |
119 | +++ src/webcatalog/schema.py 2013-03-01 12:52:22 +0000 |
120 | @@ -38,6 +38,8 @@ |
121 | 'publishers': [{ |
122 | 'type': 'datedir', |
123 | 'error_dir': 'oopses', |
124 | + 'inherit_id': True, |
125 | + 'new_only': False, |
126 | }], |
127 | }) |
128 | |
129 | |
130 | === modified file 'src/webcatalog/templates/500.html' |
131 | --- src/webcatalog/templates/500.html 2012-08-23 14:59:18 +0000 |
132 | +++ src/webcatalog/templates/500.html 2013-03-01 12:52:22 +0000 |
133 | @@ -1,10 +1,11 @@ |
134 | -{% extends "light/index.1col.html" %} |
135 | +{% extends "webcatalog/base.html" %} |
136 | {% load i18n %} |
137 | |
138 | {% block title %}{% trans 'Oops!' %}{% endblock %} |
139 | |
140 | {% block content %} |
141 | -<h1>{% trans 'Server Error <em>(500)</em>' %}</h1> |
142 | +<h1>{% trans 'Server Error' %}</h1> |
143 | {% if errormsg %}<p>{{ errormsg }}</p>{% endif %} |
144 | <p>{% blocktrans %}An error occurred which prevented the page you requested from loading. If this problem continues, please consider reporting this problem by sending an e-mail to <a href="mailto:webmaster@canonical.com">webmaster@canonical.com</a>{% endblocktrans %}</p> |
145 | +<p id="oops-id">{{ oops_id }}</p> |
146 | {% endblock %} |
147 | |
148 | === modified file 'src/webcatalog/tests/test_oops_wsgi_config.py' |
149 | --- src/webcatalog/tests/test_oops_wsgi_config.py 2012-08-23 14:59:18 +0000 |
150 | +++ src/webcatalog/tests/test_oops_wsgi_config.py 2013-03-01 12:52:22 +0000 |
151 | @@ -26,6 +26,7 @@ |
152 | |
153 | import bson |
154 | import os |
155 | +import re |
156 | from shutil import rmtree |
157 | from tempfile import mkdtemp |
158 | |
159 | @@ -67,13 +68,15 @@ |
160 | |
161 | def request_wsgi_response(self, environ): |
162 | start_response = Mock() |
163 | - with patch_settings(OOPSES={ |
164 | + oops_config = { |
165 | 'publishers': [{ |
166 | 'type': 'datedir', |
167 | 'error_dir': self.oops_dir, |
168 | - 'instance_id': 'dev', |
169 | + 'inherit_id': True, |
170 | + 'new_only': False, |
171 | }], |
172 | - }): |
173 | + } |
174 | + with patch_settings(OOPSES=oops_config): |
175 | app = make_app() |
176 | response = app(environ, start_response).next() |
177 | return response |
178 | @@ -105,6 +108,11 @@ |
179 | def test_oops_on_disk_on_error(self): |
180 | response = self.request_with_exception() |
181 | |
182 | - self.assertIn('Server Error <em>(500)</em>', response) |
183 | + self.assertIn('Server Error', response) |
184 | + oops_id_re = re.compile('<p id="oops-id">(OOPS-[0-9a-f]{32})</p>') |
185 | + match = oops_id_re.search(response) |
186 | + self.assertTrue(match) |
187 | + oops_id = match.groups()[0] |
188 | oops_data = self.read_oops_from_disk() |
189 | self.assertEqual('ZeroDivisionError', oops_data['type']) |
190 | + self.assertEqual(oops_id, oops_data['id']) |
191 | |
192 | === modified file 'src/webcatalog/urls.py' |
193 | --- src/webcatalog/urls.py 2012-07-02 21:25:30 +0000 |
194 | +++ src/webcatalog/urls.py 2013-03-01 12:52:22 +0000 |
195 | @@ -63,4 +63,5 @@ |
196 | name='wc-task-status'), |
197 | |
198 | (r'^api/', include('webcatalog.api.urls')), |
199 | + url(r'^die/$', 'die', name='debug-die'), |
200 | ) |
201 | |
202 | === modified file 'src/webcatalog/views.py' |
203 | --- src/webcatalog/views.py 2012-09-06 10:38:18 +0000 |
204 | +++ src/webcatalog/views.py 2013-03-01 12:52:22 +0000 |
205 | @@ -34,7 +34,6 @@ |
206 | from django.http import ( |
207 | HttpResponse, |
208 | HttpResponseForbidden, |
209 | - HttpResponseNotFound, |
210 | HttpResponseRedirect, |
211 | ) |
212 | from django.shortcuts import ( |
213 | @@ -42,7 +41,10 @@ |
214 | render_to_response, |
215 | ) |
216 | from django.template import RequestContext |
217 | -from django.template.loader import render_to_string |
218 | +from django.template.loader import ( |
219 | + render_to_string, |
220 | + get_template, |
221 | +) |
222 | from django.utils.translation import ugettext as _ |
223 | from django.views.decorators.cache import never_cache |
224 | from django.views.decorators.http import require_GET |
225 | @@ -346,3 +348,57 @@ |
226 | def forbidden(request): |
227 | return HttpResponseForbidden( |
228 | render_to_string('forbidden.html', RequestContext(request))) |
229 | + |
230 | + |
231 | +class ErrorPage(object): |
232 | + def __init__(self, status, errormsg=None): |
233 | + self.template_name = '%s.html' % status |
234 | + self.status_code = status |
235 | + self.errormsg = errormsg |
236 | + |
237 | + def __call__(self, request): |
238 | + accept = request.META.get('ACCEPT', '') |
239 | + if 'application/json' in accept and not 'text/html' in accept: |
240 | + return self.render_json_error(request) |
241 | + else: |
242 | + return self.render_html_error(request) |
243 | + |
244 | + def render_html_error(self, request): |
245 | + oops_id = self.get_oops_id(request) |
246 | + template = get_template(self.template_name) |
247 | + atts = { |
248 | + 'oops_id': oops_id, |
249 | + 'errormsg': self.errormsg, |
250 | + } |
251 | + context = RequestContext(request, atts) |
252 | + return HttpResponse(template.render(context), status=self.status_code) |
253 | + |
254 | + def render_json_error(self, request): |
255 | + oops_id = self.get_oops_id(request) |
256 | + oops_context = request.META.get('oops.context', {}) |
257 | + exc_info = oops_context.get('exc_info', (None, None, None)) |
258 | + exception_type = unicode(exc_info[0]) |
259 | + response = json.dumps({ |
260 | + 'oops_id': oops_id, |
261 | + 'error_type': exception_type, |
262 | + 'error_msg': "An error occurred and has been logged.", |
263 | + }) |
264 | + return HttpResponse(response, status=self.status_code, |
265 | + content_type='application/json') |
266 | + |
267 | + def get_oops_id(self, request): |
268 | + oops_report = request.environ.get('oops.report', {}) |
269 | + return oops_report.get('id', 'OOPS ID not available.') |
270 | + |
271 | + |
272 | +server_error = ErrorPage(500) |
273 | +page_not_found = ErrorPage(404) |
274 | + |
275 | + |
276 | +class ArtificialOopsException(Exception): |
277 | + pass |
278 | + |
279 | + |
280 | +def die(request): |
281 | + """Just trigger an oops report""" |
282 | + raise ArtificialOopsException() |
283 | |
284 | === modified file 'src/webcatalog/wsgi.py' |
285 | --- src/webcatalog/wsgi.py 2012-08-23 14:59:18 +0000 |
286 | +++ src/webcatalog/wsgi.py 2013-03-01 12:52:22 +0000 |
287 | @@ -23,8 +23,40 @@ |
288 | import oops_wsgi.django |
289 | import os |
290 | import platform |
291 | +import uuid |
292 | |
293 | from django.conf import settings |
294 | +from django.core.handlers import wsgi |
295 | + |
296 | + |
297 | +class EagerOOPSWSGIHandler(wsgi.WSGIHandler): |
298 | + """Pre-generates an oops id on exception.""" |
299 | + def handle_uncaught_exception(self, request, resolver, exc_info): |
300 | + # This will pass the exception information back to oops. It |
301 | + # should not be necessary once |
302 | + # https://code.djangoproject.com/ticket/16674 has been merged. |
303 | + if 'oops.context' in request.environ: |
304 | + request.environ['oops.context']['exc_info'] = exc_info |
305 | + # Generate OOPS id early so that we can render the error page right |
306 | + # away and not depend on passing thread locals around. |
307 | + if 'oops.report' in request.environ: |
308 | + unique_id = uuid.uuid4().hex |
309 | + request.environ['oops.report']['id'] = "OOPS-%s" % unique_id |
310 | + |
311 | + return super(EagerOOPSWSGIHandler, self).handle_uncaught_exception( |
312 | + request, resolver, exc_info) |
313 | + |
314 | + def __call__(self, environ, start_response): |
315 | + def start_response_with_exc_info(status, headers, exc_info=None): |
316 | + """Custom start_response callback for wsgi.""" |
317 | + # This will pass the exception information back to oops_wgi. It |
318 | + # should not be necessary once |
319 | + # https://code.djangoproject.com/ticket/16674 has been merged. |
320 | + if exc_info is None: |
321 | + exc_info = environ['oops.context'].get('exc_info', None) |
322 | + return start_response(status, headers, exc_info) |
323 | + return super(EagerOOPSWSGIHandler, self).__call__( |
324 | + environ, start_response_with_exc_info) |
325 | |
326 | |
327 | def make_app(): |
328 | @@ -38,9 +70,7 @@ |
329 | if logging_config: |
330 | logging.config.fileConfig(logging_config) |
331 | |
332 | - # This is needed to workaround a bug in Django, see the file it is |
333 | - # implemented in for more details. |
334 | - non_oops_app = oops_wsgi.django.OOPSWSGIHandler() |
335 | + non_oops_app = EagerOOPSWSGIHandler() |
336 | |
337 | config = oops_dictconfig.config_from_dict(settings.OOPSES) |
338 | oops_wsgi.install_hooks(config) |
Why there's render_html_error but no render_json_error (which should be extracted from __call__)?