Merge lp:~jtv/maas/oops into lp:maas/trunk

[Jeroen T. Vermeulen]

Basic oops integration.

Basic oops integration.

This enables basic oops logging for MaaS: if django raises an exception while handling a request, log it as an oops. By default, production oopses go into /var/log/maas/<date> (our packaging will have to create /var/log/maas with appropriate ownership) and development oopses go into ./logs (to maintain isolation between branches).

Don't expect to see familiar Launchpad-style oops pages just yet. Two things are still missing:

 * In debug mode, django catches exceptions and displays its own error page, with essentially the same information. Which is probably fine, but the error doesn't bubble up out of django and so no oops is stored.

 * There's no display support for oopses. You may want to install and use python-oops-tools yourself to work with the oops files.

Jeroen

[Gavin Panella (allenap)]

Looks great :)

[1]

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py 2012-01-19 13:56:36 +0000
+++ src/maas/settings.py 2012-01-19 15:28:18 +0000
@@ -20,6 +20,9 @@

 MANAGERS = ADMINS

+# Location where python-oops should store errors.
+OOPS_REPOSITORY = '/var/log/maas'
+

Would it be worth setting this in production.py and using something
like "/does/not/exist" in settings.py?

[2]

=== added file 'src/maasserver/management/commands/runserver.py'

What happens if we get another app in this Django project? Does Django
correctly execute the runserver command for each application in in the
project, or will this break?

[3]

Can you use the template from templates/module.py and run
utilities/format-new-and-modified-imports?

[4]

+ wsgi_handler = BaseRunserverCommand.get_handler(self, *args, **kwargs)

Use super()?

review: Approve

[Julian Edwards (julian-edwards)]

+ oops_config = Config()

I think this would scan better if you did:

import oops
oops_config = oops.Config()

because I was left wondering what Config() was a config for....

Otherwise Gavin's comments capture everything I would have said, and more :)

review: Approve

[lp:~jtv/maas/oops]

30. By Jeroen T. Vermeulen on 2012-01-19

Review changes.

[Jeroen T. Vermeulen (jtv)]

> === modified file 'src/maas/settings.py'
> --- src/maas/settings.py 2012-01-19 13:56:36 +0000
> +++ src/maas/settings.py 2012-01-19 15:28:18 +0000
> @@ -20,6 +20,9 @@
>
> MANAGERS = ADMINS
>
> +# Location where python-oops should store errors.
> +OOPS_REPOSITORY = '/var/log/maas'
> +
>
> Would it be worth setting this in production.py and using something
> like "/does/not/exist" in settings.py?

Not very nice; at any rate the directory gets created. I did add it to production.py, and made the default local. I considered None, but that would just produce an obscure error on top of an app failure.

> === added file 'src/maasserver/management/commands/runserver.py'
>
> What happens if we get another app in this Django project? Does Django
> correctly execute the runserver command for each application in in the
> project, or will this break?

I think that, assuming we wanted the same oops-tools setup, we'd want to move common code out to a shared location somewhere and give that app its own runserver.py.

Not something I'm worried about right now though.

> Can you use the template from templates/module.py and run
> utilities/format-new-and-modified-imports?

Done. Also for the other command modules.

> [4]
>
> + wsgi_handler = BaseRunserverCommand.get_handler(self, *args,
> **kwargs)
>
> Use super()?

I worried about super() being too clever for this use-case and surprising me in some way, but okay, I'll give it a whirl.

Jeroen

[lp:~jtv/maas/oops]

31. By Jeroen T. Vermeulen on 2012-01-19

Review change: super().

32. By Jeroen T. Vermeulen on 2012-01-19

Review change.

=== modified file 'buildout.cfg'
--- buildout.cfg	2012-01-18 16:48:01 +0000
+++ buildout.cfg	2012-01-19 16:53:59 +0000
@@ -25,6 +25,9 @@
   django-debug-toolbar
   django-piston
   fixtures
+  oops
+  oops-datedir-repo
+  oops-wsgi
   psycopg2
   rabbitfixture
   South

=== modified file 'src/maas/development.py'
--- src/maas/development.py	2012-01-19 13:56:36 +0000
+++ src/maas/development.py	2012-01-19 16:53:59 +0000
@@ -19,6 +19,9 @@
 # cluster is running in the branch.
 TEST_RUNNER = 'maas.testing.runner.TestRunner'
 
+# Location where python-oops should store errors.
+OOPS_REPOSITORY = 'logs'
+
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
 YUI_DEBUG = DEBUG

=== modified file 'src/maas/production.py'
--- src/maas/production.py	2012-01-19 13:56:36 +0000
+++ src/maas/production.py	2012-01-19 16:53:59 +0000
@@ -12,3 +12,6 @@
 __all__ = []
 
 from maas.settings import *
+
+# Location where python-oops should store errors.
+OOPS_REPOSITORY = '/var/log/maas'

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-01-19 13:56:36 +0000
+++ src/maas/settings.py	2012-01-19 16:53:59 +0000
@@ -20,6 +20,9 @@
 
 MANAGERS = ADMINS
 
+# Location where python-oops should store errors.
+OOPS_REPOSITORY = 'logs'
+
 DEBUG = False
 TEMPLATE_DEBUG = DEBUG
 YUI_DEBUG = DEBUG

=== modified file 'src/maasserver/management/commands/deletedb.py'
--- src/maasserver/management/commands/deletedb.py	2012-01-19 12:27:11 +0000
+++ src/maasserver/management/commands/deletedb.py	2012-01-19 16:53:59 +0000
@@ -1,6 +1,20 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import print_function
+
 from subprocess import check_call
 
-from django.core.management.base import BaseCommand, CommandError
+from django.core.management.base import (
+    BaseCommand,
+    CommandError,
+    )
+
+
+"""Django command: stop and delete the local database cluster."""
+
+__metaclass__ = type
+__all__ = ['Command']
 
 
 class Command(BaseCommand):

=== modified file 'src/maasserver/management/commands/query.py'
--- src/maasserver/management/commands/query.py	2012-01-19 12:27:11 +0000
+++ src/maasserver/management/commands/query.py	2012-01-19 16:53:59 +0000
@@ -1,6 +1,20 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import print_function
+
 from subprocess import check_call
 
-from django.core.management.base import BaseCommand, CommandError
+from django.core.management.base import (
+    BaseCommand,
+    CommandError,
+    )
+
+
+"""Django command: access the development database directly in SQL."""
+
+__metaclass__ = type
+__all__ = ['Command']
 
 
 class Command(BaseCommand):
@@ -8,7 +22,7 @@
 
     Executes an SQL statement given on the command line, or opens an SQL
     shell if no statement was given.
-    """ 
+    """
 
     args = "[SQL statement]"
     help = "Access the database directly in SQL."

=== added file 'src/maasserver/management/commands/runserver.py'
--- src/maasserver/management/commands/runserver.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/management/commands/runserver.py	2012-01-19 16:53:59 +0000
@@ -0,0 +1,39 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import print_function
+
+from django.conf import settings
+from django.core.management.commands.runserver import BaseRunserverCommand
+import oops
+from oops_datedir_repo import DateDirRepo
+from oops_wsgi import (
+    install_hooks,
+    make_app,
+    )
+
+
+"""Django command: run the server.  Overrides the default implementation."""
+
+__metaclass__ = type
+__all__ = ['Command']
+
+
+class Command(BaseRunserverCommand):
+    """Customized "runserver" command that wraps the WSGI handler."""
+
+    def get_handler(self, *args, **kwargs):
+        """Overridable from `BaseRunserverCommand`: Obtain a WSGI handler."""
+        wsgi_handler = super(Command, self).get_handler(self, *args, **kwargs)
+
+        # Wrap the WSGI handler in an oops handler.  This catches (most)
+        # exceptions bubbling up out of the app, and stores them as
+        # oopses in the directory specified by the OOPS_REPOSITORY
+        # configuration setting.
+        # Django's debug mode causes it to handle exceptions itself, so
+        # don't expect oopses when DEBUG is set to True.
+        oops_config = oops.Config()
+        oops_repository = DateDirRepo(settings.OOPS_REPOSITORY, 'maasserver')
+        oops_config.publishers.append(oops_repository.publish)
+        install_hooks(oops_config)
+        return make_app(wsgi_handler, oops_config)