launchpad-reviewers team mailing list archive

[Colin Watson <mp+423447@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:process-upload-timelines into launchpad:master.

Commit message:
Make process-upload use a per-upload OOPS timeline

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/423447

Otherwise the timeline covers the entire script run, which is bloaty and not very interesting.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:process-upload-timelines into launchpad:master.

diff --git a/lib/lp/archiveuploader/tests/test_uploadprocessor.py b/lib/lp/archiveuploader/tests/test_uploadprocessor.py
index 2b5be80..11e739d 100644
--- a/lib/lp/archiveuploader/tests/test_uploadprocessor.py
+++ b/lib/lp/archiveuploader/tests/test_uploadprocessor.py
@@ -16,6 +16,7 @@ import tempfile
 from fixtures import MonkeyPatch
 import six
 from storm.locals import Store
+from testtools.matchers import LessThan
 from zope.component import (
     getGlobalSiteManager,
     getUtility,
@@ -1409,6 +1410,31 @@ class TestUploadProcessor(TestUploadProcessorBase):
         self.assertEqual('SomeException', error_report['type'])
         self.assertIn("I am an explanation", error_report['tb_text'])
 
+    def testOopsTimeline(self):
+        """Each upload has an independent OOPS timeline."""
+        self.options.builds = False
+        processor = self.getUploadProcessor(self.layer.txn)
+
+        self.queueUpload("bar_1.0-1")
+        self.queueUpload("bar_1.0-2")
+
+        # Inject an unhandled exception into the upload processor.
+        class SomeException(Exception):
+            pass
+        self.useFixture(
+            MonkeyPatch(
+                "lp.archiveuploader.nascentupload.NascentUpload."
+                "from_changesfile_path",
+                FakeMethod(failure=SomeException("I am an explanation."))))
+
+        processor.processUploadQueue()
+
+        self.assertEqual(2, len(self.oopses))
+        # If we aren't resetting the timeline between uploads, it will be
+        # much longer.
+        self.assertThat(len(self.oopses[0]["timeline"]), LessThan(5))
+        self.assertThat(len(self.oopses[1]["timeline"]), LessThan(5))
+
     def testLZMADebUpload(self):
         """Make sure that data files compressed with lzma in Debs work.
 
diff --git a/lib/lp/archiveuploader/uploadprocessor.py b/lib/lp/archiveuploader/uploadprocessor.py
index 7782783..b1f20f9 100644
--- a/lib/lp/archiveuploader/uploadprocessor.py
+++ b/lib/lp/archiveuploader/uploadprocessor.py
@@ -78,6 +78,10 @@ from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.database.sqlobject import SQLObjectNotFound
 from lp.services.log.logger import BufferLogger
+from lp.services.webapp.adapter import (
+    clear_request_started,
+    set_request_started,
+    )
 from lp.services.webapp.errorlog import (
     ErrorReportingUtility,
     ScriptRequest,
@@ -195,12 +199,15 @@ class UploadProcessor:
                     self.log.debug("Skipping %s -- does not match %s" % (
                         upload, leaf_name))
                     continue
+                set_request_started(enable_timeout=False)
                 try:
                     handler = UploadHandler.forProcessor(self, fsroot, upload)
                 except CannotGetBuild as e:
                     self.log.warning(e)
                 else:
                     handler.process()
+                finally:
+                    clear_request_started()
         finally:
             self.log.debug("Rolling back any remaining transactions.")
             self.ztm.abort()