Launchpad itself

Merge lp:~mwhudson/launchpad/read-only-xmlrpc-bug-403281 into lp:launchpad

  1. read-only-xmlrpc-bug-403281
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Francis J. Lacoste
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~mwhudson/launchpad/read-only-xmlrpc-bug-403281
Merge into: lp:launchpad
Diff against target: 98 lines (+37/-6)
2 files modified
lib/canonical/launchpad/webapp/publication.py (+3/-5)
lib/canonical/launchpad/webapp/tests/test_publication.py (+34/-1)
To merge this branch: bzr merge lp:~mwhudson/launchpad/read-only-xmlrpc-bug-403281
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical Approve
Review via email: mp+20707@code.launchpad.net

Commit message

Don't add read-only warnings to requests that don't support notifications

Description of the change

Hi there.

This branch fixes bug 403281 which causes loads of oopses during rollouts.

As discussed, I check for failure to adapt by passing a default argument to the attempt to adapt and do nothing if adaption fails. I also remove a try:/except: block that *may* have been a botched attempt to do the same thing.

Cheers,
mwh

To post a comment you must log in.
Revision history for this message
Francis J. Lacoste (flacoste) wrote :

Add comments to the tests and this is good to go!

review: Approve
Revision history for this message
Francis J. Lacoste (flacoste) wrote :

Can you land this on db-devel as release-critical now. That will save us some OOPSes during next roll-out :-)

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/publication.py'
2--- lib/canonical/launchpad/webapp/publication.py 2010-01-30 06:47:36 +0000
3+++ lib/canonical/launchpad/webapp/publication.py 2010-03-04 23:03:29 +0000
4@@ -34,7 +34,6 @@
5 from zope.app.publication.interfaces import BeforeTraverseEvent
6 from zope.app.security.interfaces import IUnauthenticatedPrincipal
7 from zope.component import getUtility, queryMultiAdapter
8-from zope.component.interfaces import ComponentLookupError
9 from zope.error.interfaces import IErrorReportingUtility
10 from zope.event import notify
11 from zope.interface import implements, providedBy
12@@ -210,8 +209,9 @@
13 def maybeNotifyReadOnlyMode(self, request):
14 """Hook to notify about read-only mode."""
15 if is_read_only():
16- try:
17- INotificationResponse(request).addWarningNotification(
18+ notification_response = INotificationResponse(request, None)
19+ if notification_response is not None:
20+ notification_response.addWarningNotification(
21 structured("""
22 Launchpad is undergoing maintenance and is in
23 read-only mode. <i>You cannot make any
24@@ -219,8 +219,6 @@
25 href="http://blog.launchpad.net/maintenance">Launchpad
26 Blog</a> for details.
27 """))
28- except ComponentLookupError:
29- pass
30
31 def getPrincipal(self, request):
32 """Return the authenticated principal for this request.
33
34=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
35--- lib/canonical/launchpad/webapp/tests/test_publication.py 2010-01-27 11:25:20 +0000
36+++ lib/canonical/launchpad/webapp/tests/test_publication.py 2010-03-04 23:03:29 +0000
37@@ -5,6 +5,7 @@
38
39 __metaclass__ = type
40
41+import logging
42 import sys
43 import unittest
44
45@@ -33,7 +34,7 @@
46 is_browser, LaunchpadBrowserPublication)
47 from canonical.launchpad.webapp.servers import (
48 LaunchpadTestRequest, WebServicePublication)
49-from canonical.testing import DatabaseFunctionalLayer
50+from canonical.testing import DatabaseFunctionalLayer, FunctionalLayer
51 from lp.testing import TestCase, TestCaseWithFactory
52
53
54@@ -61,6 +62,7 @@
55 publication.callTraversalHooks(request, obj2)
56 self.assertEquals(request.traversed_objects, [obj1])
57
58+
59 class TestReadOnlyModeSwitches(TestCase):
60 # At the beginning of every request (in publication.beforeTraversal()), we
61 # check to see if we've changed from/to read-only/read-write and if there
62@@ -153,6 +155,37 @@
63 master._connection._database.dsn_without_user.strip())
64
65
66+class TestReadOnlyNotifications(TestCase):
67+ """Tests for `LaunchpadBrowserPublication.maybeNotifyReadOnlyMode`."""
68+
69+ layer = FunctionalLayer
70+
71+ def setUp(self):
72+ TestCase.setUp(self)
73+ touch_read_only_file()
74+ self.addCleanup(remove_read_only_file, assert_mode_switch=False)
75+
76+ def test_notification(self):
77+ # In read-only mode, maybeNotifyReadOnlyMode adds a warning that
78+ # changes cannot be made to every request that supports notifications.
79+ publication = LaunchpadBrowserPublication(None)
80+ request = LaunchpadTestRequest()
81+ publication.maybeNotifyReadOnlyMode(request)
82+ self.assertEqual(1, len(request.notifications))
83+ notification = request.notifications[0]
84+ self.assertEqual(logging.WARNING, notification.level)
85+ self.assertTrue('read-only mode' in notification.message)
86+
87+ def test_notification_xmlrpc(self):
88+ # Even in read-only mode, maybeNotifyReadOnlyMode doesn't try to add a
89+ # notification to a request that doesn't support notifications.
90+ from canonical.launchpad.webapp.servers import PublicXMLRPCRequest
91+ publication = LaunchpadBrowserPublication(None)
92+ request = PublicXMLRPCRequest(None, {})
93+ # This is just assertNotRaises
94+ publication.maybeNotifyReadOnlyMode(request)
95+
96+
97 class TestWebServicePublication(TestCaseWithFactory):
98 layer = DatabaseFunctionalLayer
99