2010-09-14 Eric Seidel <eric@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Sep 2010 22:43:32 +0000 (22:43 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Sep 2010 22:43:32 +0000 (22:43 +0000)
        Reviewed by Adam Barth.

        Make it possible to run more than one commit-queue instance
        https://bugs.webkit.org/show_bug.cgi?id=45786

        Mostly we need to make sure the two (or more) instances get
        different patches to work on.  To do this, I re-worked
        the code responsible for getting the next work item to
        round trip through the status server.  The status server only
        vends patches from the work items list, only if those patches
        have not had status reported for them in the last hour.

        This is another step towards making all queues go through the
        status server, thus making it possible to run more than one
        instance of various EWS bots (as requested in bug 44292).

        The webkitpy changes are already covered by existing unit tests.
        The QueueStatusSever sadly has no testing infrastructure yet. :(

        * QueueStatusServer/handlers/nextpatch.py: Added.
        * QueueStatusServer/index.yaml:
        * QueueStatusServer/main.py:
        * Scripts/webkitpy/tool/commands/queues.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@67572 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/QueueStatusServer/handlers/nextpatch.py [new file with mode: 0644]
WebKitTools/QueueStatusServer/index.yaml
WebKitTools/QueueStatusServer/main.py
WebKitTools/Scripts/webkitpy/common/net/statusserver.py
WebKitTools/Scripts/webkitpy/tool/commands/queues.py
WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
WebKitTools/Scripts/webkitpy/tool/mocktool.py

index cacd7a3206f2975162b6fcdb13174a16ecc3b499..f37cad92867d711e3c635b7ccce3b49baad7bd92 100644 (file)
@@ -1,3 +1,29 @@
+2010-09-14  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        Make it possible to run more than one commit-queue instance
+        https://bugs.webkit.org/show_bug.cgi?id=45786
+
+        Mostly we need to make sure the two (or more) instances get
+        different patches to work on.  To do this, I re-worked
+        the code responsible for getting the next work item to
+        round trip through the status server.  The status server only
+        vends patches from the work items list, only if those patches
+        have not had status reported for them in the last hour.
+
+        This is another step towards making all queues go through the
+        status server, thus making it possible to run more than one
+        instance of various EWS bots (as requested in bug 44292).
+
+        The webkitpy changes are already covered by existing unit tests.
+        The QueueStatusSever sadly has no testing infrastructure yet. :(
+
+        * QueueStatusServer/handlers/nextpatch.py: Added.
+        * QueueStatusServer/index.yaml:
+        * QueueStatusServer/main.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+
 2010-09-15  Mihai Parparita  <mihaip@chromium.org>
 
         Reviewed by Tony Chang.
diff --git a/WebKitTools/QueueStatusServer/handlers/nextpatch.py b/WebKitTools/QueueStatusServer/handlers/nextpatch.py
new file mode 100644 (file)
index 0000000..62c70a5
--- /dev/null
@@ -0,0 +1,56 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from google.appengine.ext import webapp
+
+from model.workitems import WorkItems
+from model import queuestatus
+
+from datetime import datetime, timedelta
+
+
+class NextPatch(webapp.RequestHandler):
+    def _get_next_patch_id(self, queue_name):
+        work_items = WorkItems.all().filter("queue_name =", queue_name).get()
+        if not work_items:
+            return None
+        one_hour_ago = datetime.now() - timedelta(minutes=60)
+        statuses = queuestatus.QueueStatus.all().filter("queue_name =", queue_name).filter("date >", one_hour_ago).fetch(15)
+        active_patch_ids = set([status.active_patch_id for status in statuses])
+        for item_id in work_items.item_ids:
+            if item_id not in active_patch_ids:
+                return item_id
+        # Either there were no work items, or they're all active.
+        return None
+
+    def get(self, queue_name):
+        patch_id = self._get_next_patch_id(queue_name)
+        if not patch_id:
+            self.error(404)
+            return
+        self.response.out.write(patch_id)
index 94b6692e24cd0479d309aadf38caef34f619e23a..6c9a3b29e3e62e6423374ddfeaa21b18d6ffa009 100644 (file)
@@ -23,6 +23,11 @@ indexes:
   - name: date
     direction: desc
 
+- kind: QueueStatus
+  properties:
+  - name: queue_name
+  - name: date
+
 - kind: QueueStatus
   properties:
   - name: queue_name
index e550dc5f4553097b2f015512ccfca1ba4bd1a620..93227ca801412ceb6cc55c39b9f4b963dce2dee9 100644 (file)
@@ -35,6 +35,7 @@ from google.appengine.ext.webapp.util import run_wsgi_app
 
 from handlers.dashboard import Dashboard
 from handlers.gc import GC
+from handlers.nextpatch import NextPatch
 from handlers.patch import Patch
 from handlers.patchstatus import PatchStatus
 from handlers.queuestatus import QueueStatus
@@ -59,6 +60,7 @@ routes = [
     (r'/status-bubble/(.*)', StatusBubble),
     (r'/svn-revision/(.*)', SVNRevision),
     (r'/queue-status/(.*)', QueueStatus),
+    (r'/next-patch/(.*)', NextPatch),
     ('/update-status', UpdateStatus),
     ('/update-work-items', UpdateWorkItems),
     ('/update-svn-revision', UpdateSVNRevision),
index c8fced631ea4e0003ceae997ac06aaeaa40c956f..57390b89c314305fabac3676b5a1581ffa12474a 100644 (file)
@@ -96,6 +96,11 @@ class StatusServer:
         self.browser["work_items"] = " ".join(work_items)
         return self.browser.submit().read()
 
+    def next_work_item(self, queue_name):
+        _log.debug("Fetching next work item for %s" % queue_name)
+        patch_status_url = "%s/next-patch/%s" % (self.url, queue_name)
+        return self._fetch_url(patch_status_url)
+
     def update_work_items(self, queue_name, work_items):
         _log.debug("Recording work items: %s for %s" % (work_items, queue_name))
         return NetworkTransaction().run(lambda: self._post_work_items_to_server(queue_name, work_items))
index 638a1d489223590ae8269ee76e19fbc083b166ba..1c64e825fd6ea88fe45a3c70098f57c528894f5f 100644 (file)
@@ -144,8 +144,16 @@ class AbstractPatchQueue(AbstractQueue):
     def _update_status(self, message, patch=None, results_file=None):
         self.tool.status_server.update_status(self.name, message, patch, results_file)
 
+    # Note, eventually this will be done by a separate "feeder" queue
+    # whose job it is to poll bugzilla and feed work items into the
+    # status server for other queues to munch on.
     def _update_work_items(self, patch_ids):
         self.tool.status_server.update_work_items(self.name, patch_ids)
+        if patch_ids:
+            self.log_progress(patch_ids)
+
+    def _fetch_next_work_item(self):
+        return self.tool.status_server.next_work_item(self.name)
 
     def _did_pass(self, patch):
         self._update_status(self._pass_status, patch)
@@ -189,15 +197,20 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
             return rollout_cmp
         return cmp(a.attach_date(), b.attach_date())
 
-    def next_work_item(self):
+    def _feed_work_items_to_server(self):
+        # Grab the set of patches from bugzilla, sort them, and update the status server.
+        # Eventually this will all be done by a separate feeder queue.
         patches = self._validate_patches_in_commit_queue()
         patches = sorted(patches, self._patch_cmp)
         self._update_work_items([patch.id() for patch in patches])
-        if not patches:
+
+    def next_work_item(self):
+        self._feed_work_items_to_server()
+        # The grab the next patch to work on back from the status server.
+        patch_id = self._fetch_next_work_item()
+        if not patch_id:
             return None
-        # Only bother logging if we have patches in the queue.
-        self.log_progress([patch.id() for patch in patches])
-        return patches[0]
+        return self.tool.bugs.fetch_attachment(patch_id)
 
     def _can_build_and_test_without_patch(self):
         try:
index 56bce490c87210556b5c8cd61f25fd9240416450..1f0aefeb935d3a8cda479802e8b163f6f1232a0c 100644 (file)
@@ -121,6 +121,16 @@ class AbstractQueueTest(CommandsTest):
         self._assert_log_message(script_error, expected_output)
 
 
+class AbstractPatchQueueTest(CommandsTest):
+    def test_fetch_next_work_item(self):
+        queue = AbstractPatchQueue()
+        tool = MockTool()
+        queue.bind_to_tool(tool)
+        self.assertEquals(queue._fetch_next_work_item(), None)
+        tool.status_server = MockStatusServer(work_items=[2, 1, 3])
+        self.assertEquals(queue._fetch_next_work_item(), 2)
+
+
 class AbstractReviewQueueTest(CommandsTest):
     def test_patch_collection_delegate_methods(self):
         queue = TestReviewQueue()
index e3d36cec8800c610d33f0657d076a5fb13acfe5a..e9c81485085695c65f050644346ee8bd8b2ed699 100644 (file)
@@ -497,8 +497,9 @@ class MockIRC(object):
 
 class MockStatusServer(object):
 
-    def __init__(self):
+    def __init__(self, work_items=None):
         self.host = "example.com"
+        self._work_items = work_items or []
 
     def patch_status(self, queue_name, patch_id):
         return None
@@ -506,7 +507,13 @@ class MockStatusServer(object):
     def svn_revision(self, svn_revision):
         return None
 
+    def next_work_item(self, queue_name):
+        if not self._work_items:
+            return None
+        return self._work_items[0]
+
     def update_work_items(self, queue_name, work_items):
+        self._work_items = work_items
         log("MOCK: update_work_items: %s %s" % (queue_name, work_items))
 
     def update_status(self, queue_name, status, patch=None, results_file=None):