Changeset 60924 in webkit

Timestamp:

Jun 9, 2010 5:28:03 PM (14 years ago)

Author:

ojan@chromium.org

Message:

2010-06-09 Ojan Vafai <​ojan@chromium.org>

Reviewed by Adam Barth.

make rietveld upload faster and avoid posting to bug on errors
​https://bugs.webkit.org/show_bug.cgi?id=40389

Only grab the first item of the upload queue instead of trying
to compute the whole list upfront (which is O(n) bugzilla lookups!).

Also, don't post comments to the bug when uploading fails.

• Scripts/webkitpy/common/net/bugzilla.py:
• Scripts/webkitpy/tool/commands/queues.py:
• Scripts/webkitpy/tool/commands/queues_unittest.py:
• Scripts/webkitpy/tool/commands/stepsequence.py:
• Scripts/webkitpy/tool/mocktool.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/net/bugzilla.py (modified) (2 diffs)
• Scripts/webkitpy/tool/commands/queues.py (modified) (4 diffs)
• Scripts/webkitpy/tool/commands/queues_unittest.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/stepsequence.py (modified) (1 diff)
• Scripts/webkitpy/tool/mocktool.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-06-09  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        make rietveld upload faster and avoid posting to bug on errors
+        https://bugs.webkit.org/show_bug.cgi?id=40389
+
+        Only grab the first item of the upload queue instead of trying
+        to compute the whole list upfront (which is O(n) bugzilla lookups!).
+
+        Also, don't post comments to the bug when uploading fails.
+
+        * Scripts/webkitpy/common/net/bugzilla.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
+        * Scripts/webkitpy/tool/commands/stepsequence.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+
 2010-06-09  Ojan Vafai  <ojan@chromium.org>
 

trunk/WebKitTools/Scripts/webkitpy/common/net/bugzilla.py

@@ -206 +206 @@
 
     def in_rietveld_queue_patches(self):
-        return [patch for patch in self.patches() if patch.in_rietveld() == "?"]
+        return [patch for patch in self.patches() if (
+            patch.in_rietveld() == "?" or patch.in_rietveld() == None and patch.review() == "?")]
 
 
@@ -271 +272 @@
                     for bug_id in self.fetch_bug_ids_from_commit_queue()], [])
 
-    def _fetch_bug_ids_from_rietveld_queue(self):
+    def _fetch_rietveld_queue_patch(self, query_url):
+        bugs = self._fetch_bug_ids_advanced_query(query_url)
+        if not len(bugs):
+            return None
+
+        patches = self._fetch_bug(bugs[0]).in_rietveld_queue_patches()
+        return patches[0] if len(patches) else None
+
+    def fetch_first_patch_from_rietveld_queue(self):
         # rietveld-queue processes in-rietveld? patches and then marks them in-rietveld-/+.
         # in-rietveld? patches
         rietveld_queue_url = "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=in-rietveld%3F&order=Last+Changed"
-        in_rietveld_bugs = self._fetch_bug_ids_advanced_query(rietveld_queue_url)
+        patch = self._fetch_rietveld_queue_patch(rietveld_queue_url)
+        if patch:
+            return patch
 
         # review? patches that don't have an in-rietveld flag.
         review_queue_url = "buglist.cgi?query_format=advanced&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F&field0-1-0=flagtypes.name&type0-1-0=notsubstring&value0-1-0=in-rietveld&order=Last+Changed"
-        in_rietveld_bugs.extend(self._fetch_bug_ids_advanced_query(review_queue_url))
-
-        return in_rietveld_bugs
-
-    def fetch_patches_from_rietveld_queue(self):
-        return sum([self._fetch_bug(bug_id).in_rietveld_queue_patches()
-                    for bug_id in self._fetch_bug_ids_from_rietveld_queue()], [])
+        patch = self._fetch_rietveld_queue_patch(review_queue_url)
+        if patch:
+            return patch
+
+        return None
 
     def _fetch_bug_ids_from_review_queue(self):

trunk/WebKitTools/Scripts/webkitpy/tool/commands/queues.py

@@ -290 +290 @@
 
     # StepSequenceErrorHandler methods
+    @staticmethod
+    def _error_message_for_bug(tool, status_id, script_error):
+        if not script_error.output:
+            return script_error.message_with_output()
+        results_link = tool.status_server.results_url_for_status(status_id)
+        return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
 
     @classmethod
@@ -307 +313 @@
 
     def next_work_item(self):
-        patches = self.tool.bugs.queries.fetch_patches_from_rietveld_queue()
-        if patches:
-            return patches[0]
+        patch_id = self.tool.bugs.queries.fetch_first_patch_from_rietveld_queue()
+        if patch_id:
+            return patch_id
         self._update_status("Empty queue")
 
@@ -326 +332 @@
             raise e
 
-    def _reject_patch(self, patch, message):
-        comment_text = "Could not upload patch %s to rietveld. Rietveld is down or there's a bug in the upload bot." % patch.id()
-        self.tool.bugs.set_flag_on_attachment(patch.id(), "in-rietveld", "-", comment_text, message)
+    def _reject_patch(self, patch_id, message):
+        self.tool.bugs.set_flag_on_attachment(patch_id, "in-rietveld", "-")
 
     def handle_unexpected_error(self, patch, message):
-        self._reject_patch(patch, message)
+        self._reject_patch(patch.id(), message)
 
     # StepSequenceErrorHandler methods
@@ -338 +343 @@
     def handle_script_error(cls, tool, state, script_error):
         status_id = cls._update_status_for_script_error(tool, state, script_error)
-        cls._reject_patch(patch, cls._error_message_for_bug(tool, status_id, script_error))
+        cls._reject_patch(state["patch"].id())
 
 

trunk/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py

@@ -206 +206 @@
             "should_proceed_with_work_item": "MOCK: update_status: rietveld-upload-queue Uploading patch\n",
             "process_work_item": "MOCK: update_status: rietveld-upload-queue Pass\n",
-            "handle_unexpected_error": "MOCK setting flag 'in-rietveld' to '-' on attachment '1234' with comment 'Could not upload patch 1234 to rietveld. Rietveld is down or there's a bug in the upload bot.' and additional comment 'Mock error message'\n",
+            "handle_unexpected_error": "MOCK setting flag 'in-rietveld' to '-' on attachment '1234' with comment 'None' and additional comment 'None'\n",
         }
         self.assert_queue_outputs(RietveldUploadQueue(), expected_stderr=expected_stderr)

trunk/WebKitTools/Scripts/webkitpy/tool/commands/stepsequence.py

@@ -36 +36 @@
 
 class StepSequenceErrorHandler():
-    # FIXME: StepSequenceErrorHandler is just an interface definition.
-    # It shouldn't have any implementations.
-    # Create an intermediate class that inherits from StepSequenceErrorHandler
-    # that the appropriate queues inherit from.
-    @staticmethod
-    def _error_message_for_bug(tool, status_id, script_error):
-        if not script_error.output:
-            return script_error.message_with_output()
-        results_link = tool.status_server.results_url_for_status(status_id)
-        return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
-
     @classmethod
     def handle_script_error(cls, tool, patch, script_error):

trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

@@ -226 +226 @@
         return sum([bug.reviewed_patches() for bug in self._all_bugs()], [])
 
-    def fetch_patches_from_rietveld_queue(self):
-        return sum([bug.in_rietveld_queue_patches() for bug in self._all_bugs()], [])
+    def fetch_first_patch_from_rietveld_queue(self):
+        for bug in self._all_bugs():
+            patches = bug.in_rietveld_queue_patches()
+            if len(patches):
+                return patches[0]
+        raise Exception('No patches in the rietveld queue')
 
 # FIXME: Bugzilla is the wrong Mock-point.  Once we have a BugzillaNetwork