Colin Watson has proposed merging
~cjwatson/launchpad:pool-file-overwrite-error-enormous-oopses into
launchpad:master.
Commit message:
Suppress timeline in PoolFileOverwriteError OOPSes
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/436331
The traceback for `PoolFileOverwriteError` (raised when the publisher attempts
to overwrite an existing file in a pool with different content) tells us
everything we need to know: we get both of the checksums and the target path.
However, the OOPSes also contain an enormous timeline mainly consisting of
every SQL statement the publisher has issued as part of processing that
archive. These timelines tend to cause the OOPS processing system to back up,
and provide essentially no information of any value. Instead, just arrange for
these OOPSes to be raised with a temporary empty timeline.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad:pool-file-overwrite-error-enormous-oopses into
launchpad:master.
diff --git a/lib/lp/services/timeline/requesttimeline.py b/lib/lp/services/timeline/requesttimeline.py
index 0c1460c..7fb0332 100644
--- a/lib/lp/services/timeline/requesttimeline.py
+++ b/lib/lp/services/timeline/requesttimeline.py
@@ -6,8 +6,11 @@
__all__ = [
"get_request_timeline",
"set_request_timeline",
+ "temporary_request_timeline",
]
+from contextlib import contextmanager
+
from timeline import Timeline
# XXX RobertCollins 2010-09-01 bug=623199: Undesirable but pragmatic.
@@ -48,3 +51,20 @@ def set_request_timeline(request, timeline):
return timeline
# Disabled code path: bug 623199, ideally we would use this code path.
request.annotations["timeline"] = timeline
+
+
+@contextmanager
+def temporary_request_timeline(request):
+ """Give `request` a temporary timeline.
+
+ This is useful in contexts where we want to raise an OOPS but we know
+ that the timeline is uninteresting and may be very large.
+
+ :param request: A Zope/Launchpad request object.
+ """
+ old_timeline = get_request_timeline(request)
+ try:
+ set_request_timeline(request, Timeline())
+ yield
+ finally:
+ set_request_timeline(request, old_timeline)
diff --git a/lib/lp/services/timeline/tests/test_requesttimeline.py b/lib/lp/services/timeline/tests/test_requesttimeline.py
index 341ce37..a49ba8a 100644
--- a/lib/lp/services/timeline/tests/test_requesttimeline.py
+++ b/lib/lp/services/timeline/tests/test_requesttimeline.py
@@ -11,6 +11,7 @@ from lp.services import webapp
from lp.services.timeline.requesttimeline import (
get_request_timeline,
set_request_timeline,
+ temporary_request_timeline,
)
@@ -55,3 +56,16 @@ class TestRequestTimeline(testtools.TestCase):
self.assertEqual(timeline, get_request_timeline(req))
# Tests while adapter._local contains the timeline ---end---
+
+ def test_temporary_request_timeline(self):
+ req = TestRequest()
+ timeline = Timeline()
+ set_request_timeline(req, timeline)
+ timeline.start("test", "test").finish()
+ self.assertEqual(timeline, get_request_timeline(req))
+ self.assertEqual(1, len(timeline.actions))
+ with temporary_request_timeline(req):
+ self.assertNotEqual(timeline, get_request_timeline(req))
+ get_request_timeline(req).start("test-2", "test-2").finish()
+ self.assertEqual(timeline, get_request_timeline(req))
+ self.assertEqual(1, len(timeline.actions))
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index b2f7a9d..82eea43 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -50,6 +50,7 @@ from lp.services.database.stormexpr import IsDistinctFrom
from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.librarian.model import LibraryFileAlias, LibraryFileContent
from lp.services.propertycache import cachedproperty, get_property_cache
+from lp.services.timeline.requesttimeline import temporary_request_timeline
from lp.services.webapp.errorlog import ErrorReportingUtility, ScriptRequest
from lp.services.worlddata.model.country import Country
from lp.soyuz.adapters.proxiedsourcefiles import ProxiedSourceLibraryFileAlias
@@ -190,7 +191,8 @@ class ArchivePublisherBase:
properties = [("error-explanation", message)]
request = ScriptRequest(properties)
error_utility = ErrorReportingUtility()
- error_utility.raising(sys.exc_info(), request)
+ with temporary_request_timeline(request):
+ error_utility.raising(sys.exc_info(), request)
log.error("%s (%s)" % (message, request.oopsid))
else:
self.setPublished()
diff --git a/lib/lp/soyuz/tests/test_publishing.py b/lib/lp/soyuz/tests/test_publishing.py
index d2e041c..7812578 100644
--- a/lib/lp/soyuz/tests/test_publishing.py
+++ b/lib/lp/soyuz/tests/test_publishing.py
@@ -994,8 +994,11 @@ class TestNativePublishing(TestNativePublishingBase):
pub_source = self.getPubSource(filecontent=b"Something")
pub_source.publish(self.disk_pool, self.logger)
- # And an oops should be filed for the error.
+ # An oops should be filed for the error, but we don't include the
+ # SQL timeline; it may be very large and tells us nothing that we
+ # can't get from the error message.
self.assertEqual("PoolFileOverwriteError", self.oopses[0]["type"])
+ self.assertEqual([], self.oopses[0]["timeline"])
self.layer.commit()
self.assertEqual(pub_source.status, PackagePublishingStatus.PENDING)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
