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
=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py 2010-01-30 06:47:36 +0000
+++ lib/canonical/launchpad/webapp/publication.py 2010-03-04 23:03:29 +0000
@@ -34,7 +34,6 @@
34from zope.app.publication.interfaces import BeforeTraverseEvent34from zope.app.publication.interfaces import BeforeTraverseEvent
35from zope.app.security.interfaces import IUnauthenticatedPrincipal35from zope.app.security.interfaces import IUnauthenticatedPrincipal
36from zope.component import getUtility, queryMultiAdapter36from zope.component import getUtility, queryMultiAdapter
37from zope.component.interfaces import ComponentLookupError
38from zope.error.interfaces import IErrorReportingUtility37from zope.error.interfaces import IErrorReportingUtility
39from zope.event import notify38from zope.event import notify
40from zope.interface import implements, providedBy39from zope.interface import implements, providedBy
@@ -210,8 +209,9 @@
210 def maybeNotifyReadOnlyMode(self, request):209 def maybeNotifyReadOnlyMode(self, request):
211 """Hook to notify about read-only mode."""210 """Hook to notify about read-only mode."""
212 if is_read_only():211 if is_read_only():
213 try:212 notification_response = INotificationResponse(request, None)
214 INotificationResponse(request).addWarningNotification(213 if notification_response is not None:
214 notification_response.addWarningNotification(
215 structured("""215 structured("""
216 Launchpad is undergoing maintenance and is in216 Launchpad is undergoing maintenance and is in
217 read-only mode. <i>You cannot make any217 read-only mode. <i>You cannot make any
@@ -219,8 +219,6 @@
219 href="http://blog.launchpad.net/maintenance">Launchpad219 href="http://blog.launchpad.net/maintenance">Launchpad
220 Blog</a> for details.220 Blog</a> for details.
221 """))221 """))
222 except ComponentLookupError:
223 pass
224222
225 def getPrincipal(self, request):223 def getPrincipal(self, request):
226 """Return the authenticated principal for this request.224 """Return the authenticated principal for this request.
227225
=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
--- lib/canonical/launchpad/webapp/tests/test_publication.py 2010-01-27 11:25:20 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publication.py 2010-03-04 23:03:29 +0000
@@ -5,6 +5,7 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8import logging
8import sys9import sys
9import unittest10import unittest
1011
@@ -33,7 +34,7 @@
33 is_browser, LaunchpadBrowserPublication)34 is_browser, LaunchpadBrowserPublication)
34from canonical.launchpad.webapp.servers import (35from canonical.launchpad.webapp.servers import (
35 LaunchpadTestRequest, WebServicePublication)36 LaunchpadTestRequest, WebServicePublication)
36from canonical.testing import DatabaseFunctionalLayer37from canonical.testing import DatabaseFunctionalLayer, FunctionalLayer
37from lp.testing import TestCase, TestCaseWithFactory38from lp.testing import TestCase, TestCaseWithFactory
3839
3940
@@ -61,6 +62,7 @@
61 publication.callTraversalHooks(request, obj2)62 publication.callTraversalHooks(request, obj2)
62 self.assertEquals(request.traversed_objects, [obj1])63 self.assertEquals(request.traversed_objects, [obj1])
6364
65
64class TestReadOnlyModeSwitches(TestCase):66class TestReadOnlyModeSwitches(TestCase):
65 # At the beginning of every request (in publication.beforeTraversal()), we67 # At the beginning of every request (in publication.beforeTraversal()), we
66 # check to see if we've changed from/to read-only/read-write and if there68 # check to see if we've changed from/to read-only/read-write and if there
@@ -153,6 +155,37 @@
153 master._connection._database.dsn_without_user.strip())155 master._connection._database.dsn_without_user.strip())
154156
155157
158class TestReadOnlyNotifications(TestCase):
159 """Tests for `LaunchpadBrowserPublication.maybeNotifyReadOnlyMode`."""
160
161 layer = FunctionalLayer
162
163 def setUp(self):
164 TestCase.setUp(self)
165 touch_read_only_file()
166 self.addCleanup(remove_read_only_file, assert_mode_switch=False)
167
168 def test_notification(self):
169 # In read-only mode, maybeNotifyReadOnlyMode adds a warning that
170 # changes cannot be made to every request that supports notifications.
171 publication = LaunchpadBrowserPublication(None)
172 request = LaunchpadTestRequest()
173 publication.maybeNotifyReadOnlyMode(request)
174 self.assertEqual(1, len(request.notifications))
175 notification = request.notifications[0]
176 self.assertEqual(logging.WARNING, notification.level)
177 self.assertTrue('read-only mode' in notification.message)
178
179 def test_notification_xmlrpc(self):
180 # Even in read-only mode, maybeNotifyReadOnlyMode doesn't try to add a
181 # notification to a request that doesn't support notifications.
182 from canonical.launchpad.webapp.servers import PublicXMLRPCRequest
183 publication = LaunchpadBrowserPublication(None)
184 request = PublicXMLRPCRequest(None, {})
185 # This is just assertNotRaises
186 publication.maybeNotifyReadOnlyMode(request)
187
188
156class TestWebServicePublication(TestCaseWithFactory):189class TestWebServicePublication(TestCaseWithFactory):
157 layer = DatabaseFunctionalLayer190 layer = DatabaseFunctionalLayer
158191