Merge lp:~michael.nelson/ubuntu-webcatalog/render_404_rather_than_raise_exception into lp:ubuntu-webcatalog

[Michael Nelson]

Use oops-wsgi for errors.

Overview
========

Contrary to the name, this branch adds and configures oops-wsgi for use with ubuntu-webcatalog.

`fab test`

[James Westby (james-w)]

68 + 'instance_id': 'dev',

It is preferred not to set instance_id at all.

16 +[oops_datedir_publisher]
17 +type = datedir
18 +error_dir = /tmp/uwc-oops/
19 +instance_id = dev

Why override from the default here? Having the oopses in-tree seems
preferable to me anyway.

Other than that this looks excellent.

Thanks,

James

review: Approve

[lp:~michael.nelson/ubuntu-webcatalog/render_404_rather_than_raise_exception]

169. By Michael Nelson on 2012-08-23

REFACTOR: updated oops-dictconfig version to that used on our precise production boxes.

170. By Michael Nelson on 2012-08-23

REFACTOR: remove instance_id setting and update eg config to match the default.

=== modified file 'django_project/config/main.cfg'
--- django_project/config/main.cfg	2012-07-02 13:41:19 +0000
+++ django_project/config/main.cfg	2012-08-23 15:14:17 +0000
@@ -106,8 +106,15 @@
 canonical-losas = admin
 canonical-ca-hackers = developers
 
-[oops]
-oops_dir = /srv/%(hostname)s/staging-logs/www-oops
+[oops_wsgi]
+oopses = oops_publishers
+
+[oops_publishers] 
+publishers = oops_datedir_publisher
+
+[oops_datedir_publisher]
+type = datedir
+error_dir = oopses
 
 [webcatalog]
 serve_site_media = True
@@ -142,4 +149,4 @@
 broker_url = amqp://guest:guest@localhost:5672//
 celery_imports = webcatalog.tasks
 celery_result_backend = database
-celerybeat_scheduler = djcelery.schedulers.DatabaseScheduler
 \ No newline at end of file
+celerybeat_scheduler = djcelery.schedulers.DatabaseScheduler
 
=== modified file 'setup.py'
--- setup.py	2012-07-02 13:51:23 +0000
+++ setup.py	2012-08-23 15:14:17 +0000
@@ -52,6 +52,9 @@
         'PIL',
         'celery',
         'django-celery',
+        'oops-wsgi==0.0.10',
+        'oops-dictconfig==0.0.2',
+        'oops-datedir_repo==0.0.17',
         ],
     package_data = find_packages_data('src'),
     dependency_links = [
 
=== modified file 'src/webcatalog/schema.py'
--- src/webcatalog/schema.py	2012-08-21 13:57:11 +0000
+++ src/webcatalog/schema.py	2012-08-23 15:14:17 +0000
@@ -21,6 +21,7 @@
 import django
 from configglue import schema
 from django_configglue.schema import schemas
+from oops_dictconfig.configglue_options import OopsOption
 
 DjangoSchema = schemas.get('1.3.1')
 
@@ -32,9 +33,13 @@
     pgconnect_timeout = schema.IntOption(default=10)
     should_serve_https = schema.BoolOption()
 
-    class oops(schema.Section):
-        oops_dir = schema.StringOption(help='Absolute path to the directory'
-                                            ' oops reports will be stored in')
+    class oops_wsgi(schema.Section):
+        oopses = OopsOption(default={
+            'publishers': [{
+                'type': 'datedir',
+                'error_dir': 'oopses',
+            }],
+        })
 
     class openid(schema.Section):
         openid_sso_server_url = schema.StringOption()
 
=== modified file 'src/webcatalog/templates/500.html'
--- src/webcatalog/templates/500.html	2012-03-08 18:13:11 +0000
+++ src/webcatalog/templates/500.html	2012-08-23 15:14:17 +0000
@@ -7,7 +7,4 @@
 <h1>{% trans 'Server Error <em>(500)</em>' %}</h1>
 {% if errormsg %}<p>{{ errormsg }}</p>{% endif %}
 <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:[e-mail]">[e-mail]</a>{% endblocktrans %}</p>
-<p>
-{% blocktrans %}(Error <abbr>ID</abbr>: {{oopsid}}){% endblocktrans %}
-</p>
 {% endblock %}
 
=== modified file 'src/webcatalog/tests/__init__.py'
--- src/webcatalog/tests/__init__.py	2012-06-28 18:26:25 +0000
+++ src/webcatalog/tests/__init__.py	2012-08-23 15:14:17 +0000
@@ -25,6 +25,7 @@
 from .test_models import *
 from .test_managers import *
 from .test_migrations import *
+from .test_oops_wsgi_config import *
 from .test_pep8 import *
 from .test_preflight import *
 from .test_tasks import *
 
=== added file 'src/webcatalog/tests/test_oops_wsgi_config.py'
--- src/webcatalog/tests/test_oops_wsgi_config.py	1970-01-01 00:00:00 +0000
+++ src/webcatalog/tests/test_oops_wsgi_config.py	2012-08-23 15:14:17 +0000
@@ -0,0 +1,110 @@
+#   -*- coding: utf-8 -*-
+#   This file is part of the Ubuntu Web Catalog
+#   Copyright (C) 2011-2012 Canonical Ltd.
+#
+#   This program is free software: you can redistribute it and/or modify
+#   it under the terms of the GNU Affero General Public License as
+#   published by the Free Software Foundation, either version 3 of the
+#   License, or (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful,
+#   but WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#   GNU Affero General Public License for more details.
+#
+#   You should have received a copy of the GNU Affero General Public License
+#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Tests for the oops_wsgi setup."""
+
+from __future__ import absolute_import
+
+__metaclass__ = type
+__all__ = [
+    'OopsWsgiTestCase',
+]
+
+import bson
+import os
+from shutil import rmtree
+from tempfile import mkdtemp
+
+from django.core.cache import cache
+from django.core.urlresolvers import reverse
+from django.conf import settings
+from django.test import TestCase
+from mock import (
+    Mock,
+    patch,
+)
+from webcatalog.wsgi import make_app
+from webcatalog.tests.helpers import patch_settings
+
+
+class OopsWsgiTestCase(TestCase):
+
+    def setUp(self):
+        super(OopsWsgiTestCase, self).setUp()
+        self.oops_dir = mkdtemp()
+        self.addCleanup(rmtree, self.oops_dir)
+        self.environ = {
+            'PATH_INFO': '/',
+            'QUERY_STRING': '',
+            'REMOTE_ADDR': '127.0.0.1',
+            'REQUEST_METHOD': 'GET',
+            'SCRIPT_NAME': '',
+            'SERVER_NAME': 'testserver',
+            'SERVER_PORT': '80',
+            'SERVER_PROTOCOL': 'HTTP/1.1',
+            'wsgi.version': (1, 0),
+            'wsgi.url_scheme': 'http',
+            'wsgi.multiprocess': True,
+            'wsgi.multithread': False,
+            'wsgi.run_once': False,
+            'wsgi.input': '',
+        }
+        cache.clear()
+
+    def request_wsgi_response(self, environ):
+        start_response = Mock()
+        with patch_settings(OOPSES={
+            'publishers': [{
+                'type': 'datedir',
+                'error_dir': self.oops_dir,
+                'instance_id': 'dev',
+            }],
+        }):
+            app = make_app()
+            response = app(environ, start_response).next()
+        return response
+
+    def read_oops_from_disk(self):
+        # Helper to read in the oops written to disk.
+        # There is only one date directory in the oops directory.
+        dated_dirs = os.listdir(self.oops_dir)
+        self.assertEqual(1, len(dated_dirs))
+
+        # There is only one oops.
+        todays_oops_path = dated_dirs[0]
+        oops_list = os.listdir(self.oops_dir + '/' + todays_oops_path)
+        self.assertEqual(1, len(oops_list))
+
+        oops_path = self.oops_dir + '/' + todays_oops_path + '/' + oops_list[0]
+        with open(oops_path, "r") as oops_file:
+            oops_file_contents = oops_file.read()
+        return bson.loads(oops_file_contents)
+
+    def request_with_exception(self):
+        self.environ['PATH_INFO'] = reverse('wc-index')
+        view_fn_path = 'webcatalog.views.render_to_response'
+        with patch(view_fn_path) as mock_render:
+            mock_render.side_effect = ZeroDivisionError
+            response = self.request_wsgi_response(self.environ)
+        return response
+
+    def test_oops_on_disk_on_error(self):
+        response = self.request_with_exception()
+
+        self.assertIn('Server Error <em>(500)</em>', response)
+        oops_data = self.read_oops_from_disk()
+        self.assertEqual('ZeroDivisionError', oops_data['type'])
 
=== modified file 'src/webcatalog/wsgi.py'
--- src/webcatalog/wsgi.py	2012-06-06 17:42:04 +0000
+++ src/webcatalog/wsgi.py	2012-08-23 15:14:17 +0000
@@ -17,13 +17,13 @@
 
 """wsgi stack for the Apps Directory."""
 
+import logging.config
+import oops_dictconfig
+import oops_wsgi
+import oops_wsgi.django
 import os
-import logging.config
 import platform
 
-from canonical.oops.serializer import OOPSRFC822Serializer
-from canonical.oops.wsgi import OopsWare
-from django.core.handlers.wsgi import WSGIHandler
 from django.conf import settings
 
 
@@ -34,9 +34,16 @@
 
     default_id = ''.join(x for x in platform.node() if x.isalpha())
     appserver_id = getattr(settings, 'APPSERVER_ID', default_id)
-    logging.config.fileConfig(settings.WEBAPP_LOGGING_CONFIG)
-    oops_dir = getattr(settings, 'OOPS_DIR', '/tmp')
-    oops_app = OopsWare(WSGIHandler(), oops_dir=oops_dir,
-                        key=appserver_id, hide_meta=True)
-    oops_app.serial = OOPSRFC822Serializer(appserver_id, oops_dir, None)
+    logging_config = settings.WEBAPP_LOGGING_CONFIG
+    if logging_config:
+        logging.config.fileConfig(logging_config)
+
+    # This is needed to workaround a bug in Django, see the file it is
+    # implemented in for more details.
+    non_oops_app = oops_wsgi.django.OOPSWSGIHandler()
+
+    config = oops_dictconfig.config_from_dict(settings.OOPSES)
+    oops_wsgi.install_hooks(config)
+    oops_app = oops_wsgi.make_app(non_oops_app, config,
+                                  oops_on_status=['500'])
     return oops_app