Diff
Modified: trunk/Tools/ChangeLog (174247 => 174248)
--- trunk/Tools/ChangeLog 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/ChangeLog 2014-10-02 23:49:30 UTC (rev 174248)
@@ -1,3 +1,44 @@
+2014-10-02 Alexey Proskuryakov <a...@apple.com>
+
+ update-work-items should never delete items
+ https://bugs.webkit.org/show_bug.cgi?id=137366
+
+ Reviewed by Ryosuke Niwa.
+
+ As we don't just replace the whole list any more, indicate which items are high
+ priority, and which are not. Hight priority ones will be prepended to the queue,
+ others will be appended.
+
+ This creates a bit of unfairness in that high priority item queue becomes a LIFO.
+ But hopefully we will never have many rollouts competing like that.
+
+ * QueueStatusServer/app.yaml: Update version.
+
+ * QueueStatusServer/handlers/updateworkitems.py: Never remove items. Pass high
+ priority and regular items separately. Removed some error handling that used to
+ end up in returning status 500 - an uncaught exception does that automatically.
+
+ * QueueStatusServer/main.py: Removed unnecessary regexps from URL matching code.
+
+ * QueueStatusServer/model/workitems.py: Added a way to add high priority items.
+
+ * QueueStatusServer/templates/updateworkitems.html: Added a field for high
+ priority items.
+
+ * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
+ (BugzillaQueries._parse_attachment_ids_request_query): Removed an annoying log
+ line that complicated testing.
+
+ * Scripts/webkitpy/common/net/statusserver.py: Pass high priority items separately.
+
+ * Scripts/webkitpy/tool/bot/feeders.py: For commit queue, split high and regular
+ priority items into separate lists.
+
+ * Scripts/webkitpy/common/net/statusserver_mock.py:
+ * Scripts/webkitpy/tool/bot/feeders_unittest.py:
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ Updated tests.
+
2014-10-02 Brendan Long <b.l...@cablelabs.com>
Annoying build warnings in WTFString API tests
Modified: trunk/Tools/QueueStatusServer/app.yaml (174247 => 174248)
--- trunk/Tools/QueueStatusServer/app.yaml 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/QueueStatusServer/app.yaml 2014-10-02 23:49:30 UTC (rev 174248)
@@ -1,5 +1,5 @@
application: webkit-queues
-version: 174158 # Bugzilla bug ID of last major change
+version: 174248 # Bugzilla bug ID of last major change
runtime: python
api_version: 1
Modified: trunk/Tools/QueueStatusServer/handlers/updateworkitems.py (174247 => 174248)
--- trunk/Tools/QueueStatusServer/handlers/updateworkitems.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/QueueStatusServer/handlers/updateworkitems.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -1,4 +1,5 @@
# Copyright (C) 2013 Google Inc. All rights reserved.
+# Copyright (C) 2014 Apple Inc. All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -42,21 +43,15 @@
self.response.out.write(template.render("templates/updateworkitems.html", None))
def _parse_work_items_string(self, items_string):
- try:
- item_strings = items_string.split(" ") if items_string else []
- return map(int, item_strings)
- except ValueError:
- return None
+ item_strings = items_string.split(" ") if items_string else []
+ return map(int, item_strings)
- def _update_work_items_from_request(self, work_items):
+ def _work_items_from_request(self):
+ high_priority_items_string = self.request.get("high_priority_items")
items_string = self.request.get("work_items")
- new_work_items = self._parse_work_items_string(items_string)
- if new_work_items == None:
- self.response.out.write("Failed to parse work items: %s" % items_string)
- return False
- work_items.item_ids = new_work_items
- work_items.date = datetime.utcnow()
- return True
+ high_priority_work_items = self._parse_work_items_string(high_priority_items_string)
+ work_items = self._parse_work_items_string(items_string)
+ return high_priority_work_items, work_items
def _queue_from_request(self):
queue_name = self.request.get("queue_name")
@@ -71,17 +66,12 @@
if not queue:
self.response.set_status(500)
return
- work_items = queue.work_items()
- old_items = set(work_items.item_ids)
- success = self._update_work_items_from_request(work_items)
- if not success:
- self.response.set_status(500)
- return
- new_items = set(work_items.item_ids)
- work_items.put()
+ high_priority_items, items = self._work_items_from_request()
- for work_item in new_items - old_items:
+ # Add items that are not currently in the work queue. Never remove any items,
+ # as that should be done by the queue, feeder only adds them.
+ added_items = queue.work_items().add_work_items(high_priority_items, items)
+
+ for work_item in added_items:
RecordPatchEvent.added(work_item, queue.name())
- for work_item in old_items - new_items:
- RecordPatchEvent.stopped(work_item, queue.name())
Modified: trunk/Tools/QueueStatusServer/main.py (174247 => 174248)
--- trunk/Tools/QueueStatusServer/main.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/QueueStatusServer/main.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -63,7 +63,7 @@
('/sync-queue-logs', SyncQueueLogs),
(r'/patch-status/(.*)/(.*)', PatchStatus),
(r'/patch/(.*)', Patch),
- (r'/submit-to-ews', SubmitToEWS),
+ ('/submit-to-ews', SubmitToEWS),
(r'/results/(.*)', ShowResults),
(r'/status-bubble/(.*)', StatusBubble),
(r'/svn-revision/(.*)', SVNRevision),
@@ -73,8 +73,8 @@
(r'/queue-status/(.*)', QueueStatus),
(r'/queue-status-json/(.*)', QueueStatusJSON),
(r'/next-patch/(.*)', NextPatch),
- (r'/release-patch', ReleasePatch),
- (r'/release-lock', ReleaseLock),
+ ('/release-patch', ReleasePatch),
+ ('/release-lock', ReleaseLock),
('/update-status', UpdateStatus),
('/update-work-items', UpdateWorkItems),
('/update-svn-revision', UpdateSVNRevision),
Modified: trunk/Tools/QueueStatusServer/model/workitems.py (174247 => 174248)
--- trunk/Tools/QueueStatusServer/model/workitems.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/QueueStatusServer/model/workitems.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -1,4 +1,5 @@
# Copyright (C) 2010 Google Inc. All rights reserved.
+# Copyright (C) 2014 Apple Inc. All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are
@@ -53,17 +54,29 @@
return None
@staticmethod
- def _unguarded_add(key, attachment_id):
+ def _unguarded_add(key, high_priority_items, items):
work_items = db.get(key)
- if attachment_id in work_items.item_ids:
- return
- work_items.item_ids.append(attachment_id)
+ added_items = []
+ for item in high_priority_items[::-1]:
+ if item in work_items.item_ids:
+ continue
+ work_items.item_ids.insert(0, item)
+ added_items.insert(0, item)
+ for item in items:
+ if item in work_items.item_ids:
+ continue
+ work_items.item_ids.append(item)
+ added_items.append(item)
work_items.put()
+ return added_items
# Because this uses .key() self.is_saved() must be True or this will throw NotSavedError.
def add_work_item(self, attachment_id):
- db.run_in_transaction(self._unguarded_add, self.key(), attachment_id)
+ db.run_in_transaction(self._unguarded_add, self.key(), [], [attachment_id])
+ def add_work_items(self, high_priority_items, items):
+ return db.run_in_transaction(self._unguarded_add, self.key(), high_priority_items, items)
+
@staticmethod
def _unguarded_remove(key, attachment_id):
work_items = db.get(key)
Modified: trunk/Tools/QueueStatusServer/templates/updateworkitems.html (174247 => 174248)
--- trunk/Tools/QueueStatusServer/templates/updateworkitems.html 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/QueueStatusServer/templates/updateworkitems.html 2014-10-02 23:49:30 UTC (rev 174248)
@@ -1,8 +1,8 @@
<form name="update_work_items" enctype="multipart/form-data" method="post">
Update work items for a queue: <input name="queue_name">
<div>
- Work Items:
- <input name="work_items">
+ Work items: <input name="work_items">
+ High priority work items: <input name="high_priority_work_items">
</div>
<div><input type="submit" value="Update Work Items"></div>
</form>
Modified: trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py (174247 => 174248)
--- trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -204,7 +204,6 @@
patch_id = int(digits.search(patch_tag["href"]).group(0))
date_tag = row.find("td", text=date_format)
if date_tag and datetime.strptime(date_format.search(date_tag).group(0), "%Y-%m-%d %H:%M") < since:
- _log.info("Patch is old: %d (%s)" % (patch_id, date_tag))
continue
patch_ids.append(patch_id)
return patch_ids
Modified: trunk/Tools/Scripts/webkitpy/common/net/statusserver.py (174247 => 174248)
--- trunk/Tools/Scripts/webkitpy/common/net/statusserver.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/Scripts/webkitpy/common/net/statusserver.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -100,13 +100,15 @@
self._browser["broken_bot"] = broken_bot
return self._browser.submit().read()
- def _post_work_items_to_server(self, queue_name, work_items):
+ def _post_work_items_to_server(self, queue_name, high_priority_work_items, work_items):
update_work_items_url = "%s/update-work-items" % self.url
self._browser.open(update_work_items_url)
self._browser.select_form(name="update_work_items")
self._browser["queue_name"] = queue_name
work_items = map(unicode, work_items) # .join expects strings
self._browser["work_items"] = " ".join(work_items)
+ high_priority_work_items = map(unicode, high_priority_work_items)
+ self._browser["high_priority_work_items"] = " ".join(high_priority_work_items)
return self._browser.submit().read()
def _post_work_item_to_ews(self, attachment_id):
@@ -149,9 +151,9 @@
_log.info("Releasing lock for work item %s from %s" % (patch.id(), queue_name))
return NetworkTransaction(convert_404_to_None=True).run(lambda: self._post_release_lock(queue_name, patch))
- 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))
+ def update_work_items(self, queue_name, high_priority_work_items, work_items):
+ _log.debug("Recording work items: %s for %s" % (high_priority_work_items + work_items, queue_name))
+ return NetworkTransaction().run(lambda: self._post_work_items_to_server(queue_name, high_priority_work_items, work_items))
def update_status(self, queue_name, status, patch=None, results_file=None):
_log.info(status)
Modified: trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py (174247 => 174248)
--- trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -55,9 +55,9 @@
def release_lock(self, queue_name, patch):
_log.info("MOCK: release_lock: %s %s" % (queue_name, patch.id()))
- def update_work_items(self, queue_name, work_items):
+ def update_work_items(self, queue_name, high_priority_work_items, work_items):
self._work_items = work_items
- _log.info("MOCK: update_work_items: %s %s" % (queue_name, work_items))
+ _log.info("MOCK: update_work_items: %s %s" % (queue_name, high_priority_work_items + work_items))
def submit_to_ews(self, patch_id):
_log.info("MOCK: submit_to_ews: %s" % (patch_id))
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/feeders.py (174247 => 174248)
--- trunk/Tools/Scripts/webkitpy/tool/bot/feeders.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/feeders.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -50,19 +50,17 @@
AbstractFeeder.__init__(self, tool)
self.committer_validator = CommitterValidator(self._tool)
- def _update_work_items(self, item_ids):
- # FIXME: This is the last use of update_work_items, the commit-queue
- # should move to feeding patches one at a time like the EWS does.
- self._tool.status_server.update_work_items(self.queue_name, item_ids)
- _log.info("Feeding %s items %s" % (self.queue_name, item_ids))
-
def feed(self):
patches = self._validate_patches()
patches = self._patches_with_acceptable_review_flag(patches)
patches = sorted(patches, self._patch_cmp)
- patch_ids = [patch.id() for patch in patches]
- self._update_work_items(patch_ids)
+ high_priority_item_ids = [patch.id() for patch in patches if patch.is_rollout()]
+ item_ids = [patch.id() for patch in patches if not patch.is_rollout()]
+
+ _log.info("Feeding %s high priority items %s, regular items %s" % (self.queue_name, high_priority_item_ids, item_ids))
+ self._tool.status_server.update_work_items(self.queue_name, high_priority_item_ids, item_ids)
+
def _patches_for_bug(self, bug_id):
return self._tool.bugs.fetch_bug(bug_id).commit_queued_patches(include_invalid=True)
@@ -77,11 +75,6 @@
return self.committer_validator.patches_after_rejecting_invalid_commiters_and_reviewers(all_patches)
def _patch_cmp(self, a, b):
- # Sort first by is_rollout, then by attach_date.
- # Reversing the order so that is_rollout is first.
- rollout_cmp = cmp(b.is_rollout(), a.is_rollout())
- if rollout_cmp != 0:
- return rollout_cmp
return cmp(a.attach_date(), b.attach_date())
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py (174247 => 174248)
--- trunk/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -37,6 +37,7 @@
class FeedersTest(unittest.TestCase):
def test_commit_queue_feeder(self):
+ self.maxDiff = None
feeder = CommitQueueFeeder(MockTool())
expected_logs = """Warning, attachment 10001 on bug 50000 has invalid committer (non-commit...@example.com)
Warning, attachment 10001 on bug 50000 has invalid committer (non-commit...@example.com)
@@ -45,8 +46,8 @@
- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.
- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.'
+Feeding commit-queue high priority items [10005], regular items [10000]
MOCK: update_work_items: commit-queue [10005, 10000]
-Feeding commit-queue items [10005, 10000]
"""
OutputCapture().assert_outputs(self, feeder.feed, expected_logs=expected_logs)
@@ -56,19 +57,6 @@
attachment.attach_date = lambda: attach_date
return attachment
- def test_patch_cmp(self):
- long_ago_date = datetime(1900, 1, 21)
- recent_date = datetime(2010, 1, 21)
- attachment1 = self._mock_attachment(is_rollout=False, attach_date=recent_date)
- attachment2 = self._mock_attachment(is_rollout=False, attach_date=long_ago_date)
- attachment3 = self._mock_attachment(is_rollout=True, attach_date=recent_date)
- attachment4 = self._mock_attachment(is_rollout=True, attach_date=long_ago_date)
- attachments = [attachment1, attachment2, attachment3, attachment4]
- expected_sort = [attachment4, attachment3, attachment2, attachment1]
- queue = CommitQueueFeeder(MockTool())
- attachments.sort(queue._patch_cmp)
- self.assertEqual(attachments, expected_sort)
-
def test_patches_with_acceptable_review_flag(self):
class MockPatch(object):
def __init__(self, patch_id, review):
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (174247 => 174248)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2014-10-02 23:47:38 UTC (rev 174247)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2014-10-02 23:49:30 UTC (rev 174248)
@@ -128,6 +128,7 @@
class FeederQueueTest(QueuesTest):
def test_feeder_queue(self):
+ self.maxDiff = None
queue = TestFeederQueue()
tool = MockTool(log_executive=True)
expected_logs = {
@@ -139,8 +140,8 @@
- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.
- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.'
+Feeding commit-queue high priority items [10005], regular items [10000]
MOCK: update_work_items: commit-queue [10005, 10000]
-Feeding commit-queue items [10005, 10000]
Feeding EWS (1 r? patch, 1 new)
MOCK: submit_to_ews: 10002
""",
