Merge lp:~james-w/canonical-identity-provider/oops2 into lp:canonical-identity-provider/release

Proposed by James Westby
Status: Merged
Approved by: James Westby
Approved revision: no longer in the source branch.
Merged at revision: 734
Proposed branch: lp:~james-w/canonical-identity-provider/oops2
Merge into: lp:canonical-identity-provider/release
Diff against target: 159 lines (+74/-14)
7 files modified
.bzrignore (+1/-0)
django_project/config_dev/config/devel.cfg (+16/-0)
django_project/config_dev/django.wsgi (+16/-13)
identityprovider/schema.py (+4/-0)
identityprovider/wsgi_handler.py (+32/-0)
requirements/install.txt (+4/-0)
webui/views/errors.py (+1/-1)
To merge this branch: bzr merge lp:~james-w/canonical-identity-provider/oops2
Reviewer Review Type Date Requested Status
Ricardo Kirkner (community) Approve
Review via email: mp+153935@code.launchpad.net

Commit message

Prepare for moving to v2 of the oops stack.

Description of the change

Hi,

This is the prep that is needed for moving to v2 of the oops stack.

It's not perfect yet, as a limitation in the oops stack means we currently
have to choose between showing the user the id of their oops, and having reliable
delivery if amqp is down. I'm working on fixing that limitation, and we can
adopt that when it is ready so that we don't have to choose.

Some comments:

  * There are extra requirements that this brings. They should all be
    packaged in CAT already, so I think it should be a case of adding them
    to the dependencies packaage and getting them installed?

  * It requires some config being set to get the oopses delivered, but the
    code won't have a problem if it isn't set. How is the config delivered
    to production for SSO?

  * I didn't make use of settings.BRAND for the new oopses. I'm not sure what
    it is for. However we can adjust the reporter or the template to include
    extra information if we like. If the info is dynamic and important to have
    then I can add the code for it.

Thanks,

James

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

Oh, I renamed the wsgi symlink so that I could use it with runserver, as it needs to
import it. I can undo that if it's not correct.

Thanks,

James

Revision history for this message
James Westby (james-w) wrote :

nessita points out that sourcedeps might be preferred to pip now. Also sso is on lucid, and
the packages we have will be for precise.

There's also a question of whether requirements.txt should list all the new deps (e.g. the
amqp library that oops_amqp will use), or just the top-level ones.

Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

Approving, but noting some extra things we need to add in later

- add requirements as sourcedeps
- add tests for wsgi_handler.py
- add a way for testing wsgi.py

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (5.5 KiB)

The attempt to merge lp:~james-w/canonical-identity-provider/oops2 into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Updating download cache at dir /mnt/tarmac/cache/isd-download-cache
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/+junk/download-cache/
No revisions or tags to pull.
[localhost] local: which virtualenv
[localhost] local: /usr/bin/python /usr/bin/virtualenv --version
[localhost] local: /usr/bin/python /usr/bin/virtualenv --distribute --clear .env
Not deleting .env/bin
New python executable in .env/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
[localhost] local: dpkg -l libpq-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxml2-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxslt1-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l memcached 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l postgresql-plpython-9.1 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-m2crypto 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l swig 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l config-manager 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-egenix-mx-base-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: rm -rf M2Crypto*
[localhost] local: ln -s /usr/lib/python2.7/dist-packages/M2Crypto* .
[localhost] local: /usr/lib/config-manager/cm.py update /tmp/tmpHHZDAd
[localhost] local: . /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin/activate && make install PACKAGES="-r /mnt/tarmac/cache/canonical-identity-provider/trunk/requirements.txt"
pip install --find-links=file:///mnt/tarmac/cache/isd-download-cache --no-index pip==dev
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking pip==dev
  Running setup.py egg_info for package pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
Installing collected packages: pip
  Found existing installation: pip 1.1
    Uninstalling pip:
      Successfully uninstalled pip
  Running setup.py install for pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
    Installing pip script to /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin
    Installing pip-2.7 script to /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin
Successfully installed pip
Cleaning up...
pip install --find-links=. --no-index -r /mnt/tarmac/cache/canonical-identity-provider/trunk/requirements.txt
Ignoring indexes: ht...

Read more...

Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (8.9 KiB)

The attempt to merge lp:~james-w/canonical-identity-provider/oops2 into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Updating download cache at dir /mnt/tarmac/cache/isd-download-cache
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/+junk/download-cache/
No revisions or tags to pull.
[localhost] local: which virtualenv
[localhost] local: /usr/bin/python /usr/bin/virtualenv --version
[localhost] local: /usr/bin/python /usr/bin/virtualenv --distribute --clear .env
Not deleting .env/bin
New python executable in .env/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
[localhost] local: dpkg -l libpq-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxml2-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxslt1-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l memcached 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l postgresql-plpython-9.1 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-m2crypto 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l swig 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l config-manager 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-egenix-mx-base-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: rm -rf M2Crypto*
[localhost] local: ln -s /usr/lib/python2.7/dist-packages/M2Crypto* .
[localhost] local: /usr/lib/config-manager/cm.py update /tmp/tmpU44EE3
[localhost] local: . /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin/activate && make install PACKAGES="-r /mnt/tarmac/cache/canonical-identity-provider/trunk/requirements.txt"
pip install --find-links=file:///mnt/tarmac/cache/isd-download-cache --no-index pip==dev
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking pip==dev
  Running setup.py egg_info for package pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
Installing collected packages: pip
  Found existing installation: pip 1.1
    Uninstalling pip:
      Successfully uninstalled pip
  Running setup.py install for pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
    Installing pip script to /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin
    Installing pip-2.7 script to /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin
Successfully installed pip
Cleaning up...
pip install --find-links=. --no-index -r /mnt/tarmac/cache/canonical-identity-provider/trunk/requirements.txt
Ignoring indexes: ht...

Read more...

Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (79.0 KiB)

The attempt to merge lp:~james-w/canonical-identity-provider/oops2 into lp:canonical-identity-provider failed. Below is the output from the failed tests.

Updating download cache at dir /mnt/tarmac/cache/isd-download-cache
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~canonical-isd-hackers/+junk/download-cache/
Now on revision 16.
[localhost] local: which virtualenv
[localhost] local: /usr/bin/python /usr/bin/virtualenv --version
[localhost] local: /usr/bin/python /usr/bin/virtualenv --distribute --clear .env
Not deleting .env/bin
New python executable in .env/bin/python
Installing distribute.............................................................................................................................................................................................done.
Installing pip...............done.
[localhost] local: dpkg -l libpq-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxml2-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l libxslt1-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l memcached 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l postgresql-plpython-9.1 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-m2crypto 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l swig 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l config-manager 2> /dev/null | grep '^ii' | wc -l
[localhost] local: dpkg -l python-egenix-mx-base-dev 2> /dev/null | grep '^ii' | wc -l
[localhost] local: rm -rf M2Crypto*
[localhost] local: ln -s /usr/lib/python2.7/dist-packages/M2Crypto* .
[localhost] local: /usr/lib/config-manager/cm.py update /tmp/tmpilY6JQ
[localhost] local: . /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin/activate && make install PACKAGES="-r /mnt/tarmac/cache/canonical-identity-provider/trunk/requirements.txt"
pip install --find-links=file:///mnt/tarmac/cache/isd-download-cache --no-index pip==dev
Ignoring indexes: http://pypi.python.org/simple/
Downloading/unpacking pip==dev
  Running setup.py egg_info for package pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
Installing collected packages: pip
  Found existing installation: pip 1.1
    Uninstalling pip:
      Successfully uninstalled pip
  Running setup.py install for pip

    warning: no files found matching '*.html' under directory 'docs'
    warning: no previously-included files matching '*.txt' found under directory 'docs/_build'
    no previously-included directories found matching 'docs/_build/_sources'
    Installing pip script to /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin
    Installing pip-2.7 script to /mnt/tarmac/cache/canonical-identity-provider/trunk/.env/bin
Successfully installed pip
Cleaning up...
pip install --find-links=. --no-index -r /mnt/tarmac/cache/canonical-identity-provider/trunk/requirements.txt
Ignoring indexes: http://pypi....

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.bzrignore'
2--- .bzrignore 2013-03-05 13:51:14 +0000
3+++ .bzrignore 2013-03-19 21:11:19 +0000
4@@ -28,4 +28,5 @@
5 branches/*
6 django_project/config_dev/config/local.cfg
7 scripts/local.cfg-
8+./oopses/*
9
10
11=== modified file 'django_project/config_dev/config/devel.cfg'
12--- django_project/config_dev/config/devel.cfg 2013-03-12 19:14:34 +0000
13+++ django_project/config_dev/config/devel.cfg 2013-03-19 21:11:19 +0000
14@@ -196,3 +196,19 @@
15
16 [api]
17 api_host = http://%(hostname)s
18+
19+[oopses]
20+oopses = oops_config
21+
22+[oops_config]
23+template = oops_template
24+publishers = oops_dev_publisher
25+
26+[oops_template]
27+reporter = SSO-DEV
28+
29+[oops_dev_publisher]
30+type = datedir
31+error_dir = oopses
32+inherit_id = true
33+only_new = false
34
35=== modified file 'django_project/config_dev/django.wsgi'
36--- django_project/config_dev/django.wsgi 2013-02-22 12:16:44 +0000
37+++ django_project/config_dev/django.wsgi 2013-03-19 21:11:19 +0000
38@@ -31,16 +31,19 @@
39
40 os.environ['PGCONNECT_TIMEOUT'] = str(settings.PGCONNECT_TIMEOUT)
41
42-default_id = ''.join(x for x in platform.node() if x.isalpha())
43-appserver_id = getattr(settings, 'APPSERVER_ID', default_id)
44-appserver_id += settings.BRAND
45-
46-app = WSGIHandler()
47-
48-
49-def application(environ, start_response):
50- oops_dir = getattr(settings, 'OOPS_DIR', '/tmp')
51- serializer = 'canonical.oops.serializer.OOPSRFC822Serializer'
52- oops = OopsWare(app, oops_dir=oops_dir, key=appserver_id, hide_meta=True,
53- serializer_factory_name=serializer)
54- return oops(environ, start_response)
55+from identityprovider.wsgi_handler import OOPSWSGIHandler
56+
57+app = OOPSWSGIHandler()
58+
59+# Wrap the application in the Oops wsgi app to catch unhandled exceptions
60+# and create oops for them.
61+#
62+# First we create the config that defines what to do with the oopses.
63+import oops_dictconfig
64+from oops_wsgi import make_app, install_hooks
65+
66+config = oops_dictconfig.config_from_dict(settings.OOPSES)
67+install_hooks(config)
68+
69+# Then we wrap the django app in the oops one
70+application = make_app(app, config, oops_on_status=['500'])
71
72=== renamed symlink 'django_project/django.wsgi' => 'django_project/wsgi.py'
73=== modified file 'identityprovider/schema.py'
74--- identityprovider/schema.py 2013-03-12 19:14:34 +0000
75+++ identityprovider/schema.py 2013-03-19 21:11:19 +0000
76@@ -18,6 +18,7 @@
77 merge,
78 )
79 from django_configglue.schema import schemas
80+from oops_dictconfig.configglue_options import OopsOption
81
82 from ubuntu_sso_saml.schema import Saml2IdpSchema
83
84@@ -278,6 +279,9 @@
85 session_cookie_secure = BoolOption(default=True)
86 session_cookie_httponly = BoolOption(default=True)
87
88+ class oopses(Section):
89+ oopses = OopsOption()
90+
91
92 # merge all contrib schemas into the base schema
93 # order matters
94
95=== added file 'identityprovider/wsgi_handler.py'
96--- identityprovider/wsgi_handler.py 1970-01-01 00:00:00 +0000
97+++ identityprovider/wsgi_handler.py 2013-03-19 21:11:19 +0000
98@@ -0,0 +1,32 @@
99+import uuid
100+
101+from oops_wsgi import django as oops_wsgi_django
102+
103+
104+class OOPSWSGIHandler(oops_wsgi_django.OOPSWSGIHandler):
105+ """
106+ Custom WSGI Handler that generates oops ids when an error is encountered
107+ so the ids can be reported to the user.
108+ """
109+
110+ def handle_uncaught_exception(self, request, resolver, exc_info):
111+ # Generate OOPS id early so that we can render the error page
112+ # right
113+ # away and not depend on passing thread locals around.
114+ if 'oops.report' in request.environ:
115+ unique_id = uuid.uuid4().hex
116+ request.environ['oops.report']['id'] = "OOPS-%s" % unique_id
117+ return super(OOPSWSGIHandler, self).handle_uncaught_exception(
118+ request, resolver, exc_info)
119+
120+ def __call__(self, environ, start_response):
121+ def start_response_with_exc_info(status, headers, exc_info=None):
122+ """Custom start_response callback for wsgi."""
123+ # This will pass the exception information back to oops_wsgi. It
124+ # should not be necessary once
125+ # https://code.djangoproject.com/ticket/16674 has been merged.
126+ if exc_info is None:
127+ exc_info = environ['oops.context'].get('exc_info', None)
128+ return start_response(status, headers, exc_info)
129+ return super(OOPSWSGIHandler, self).__call__(
130+ environ, start_response_with_exc_info)
131
132=== added directory 'oopses'
133=== modified file 'requirements/install.txt'
134--- requirements/install.txt 2013-03-05 15:52:39 +0000
135+++ requirements/install.txt 2013-03-19 21:11:19 +0000
136@@ -14,6 +14,10 @@
137 M2Crypto==0.21.1
138 oath==1.0
139 oauth==1.0.1
140+oops_amqp==0.0.7
141+oops_datedir_repo==0.0.20
142+oops_dictconfig==0.0.4
143+oops_wsgi==0.0.10
144 psycopg2==2.4.1
145 pystatsd==0.1.6
146 python-memcached==1.44
147
148=== modified file 'webui/views/errors.py'
149--- webui/views/errors.py 2012-12-04 14:17:22 +0000
150+++ webui/views/errors.py 2013-03-19 21:11:19 +0000
151@@ -13,7 +13,7 @@
152
153 def __call__(self, request):
154 template = loader.get_template(self.template_name)
155- oopsid = request.META.get('OOPSID')
156+ oopsid = request.environ.get('oops.report', {}).get('id', None)
157 atts = {
158 'request': request,
159 'oopsid': oopsid,