Diff
Modified: trunk/Tools/ChangeLog (174008 => 174009)
--- trunk/Tools/ChangeLog 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/ChangeLog 2014-09-26 16:58:41 UTC (rev 174009)
@@ -1,3 +1,39 @@
+2014-09-26 Alexey Proskuryakov <[email protected]>
+
+ Get rid of Retry status in webkit-queues
+ https://bugs.webkit.org/show_bug.cgi?id=137135
+
+ Reviewed by Ryosuke Niwa.
+
+ * QueueStatusServer/config/messages.py: Removed Retry.
+
+ * QueueStatusServer/handlers/releasepatch.py: This is now straightforward, as it
+ no longer needs to check the latest status. It just always both unlocks the patch
+ and removes it from WorkItems.
+
+ * QueueStatusServer/handlers/submittoews.py: (SubmitToEWS._should_add_to_ews_queue):
+ I don't understand why we even needed to check for retries here, but now that there
+ are no retries, that code can go to /dev/null.
+
+ * QueueStatusServer/loggers/recordpatchevent.py:
+ (RecordPatchEvent.started):
+ (RecordPatchEvent.retrying): Deleted.
+ Fixed retry counting, it should work for all queues now.
+
+ * QueueStatusServer/model/queuestatus.py:
+ (QueueStatus.is_retry_request): Deleted. These are no more.
+
+ * Scripts/webkitpy/common/net/statusserver_mock.py:
+ (MockStatusServer.release_lock):
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ Did whatever it took to keep passing the tests. The particular test doesn't seem
+ quite right, but whatever.
+
+ * Scripts/webkitpy/tool/commands/queues.py:
+ (CommitQueue.process_work_item): Instead of posting a retry status, just unlock
+ and let others pick up. Also, added explicit returns for clarity.
+ (AbstractPatchQueue._did_retry): Deleted.
+
2014-09-26 Csaba Osztrogonác <[email protected]>
[EFL] Fix the gst-libav build on ARM Thumb2
Modified: trunk/Tools/QueueStatusServer/app.yaml (174008 => 174009)
--- trunk/Tools/QueueStatusServer/app.yaml 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/QueueStatusServer/app.yaml 2014-09-26 16:58:41 UTC (rev 174009)
@@ -1,5 +1,5 @@
application: webkit-queues
-version: 173979 # Bugzilla bug ID of last major change
+version: 174009 # Bugzilla bug ID of last major change
runtime: python
api_version: 1
Modified: trunk/Tools/QueueStatusServer/config/messages.py (174008 => 174009)
--- trunk/Tools/QueueStatusServer/config/messages.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/QueueStatusServer/config/messages.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -29,5 +29,4 @@
# These must be in sync with webkit-patch's AbstractQueue.
pass_status = "Pass"
fail_status = "Fail"
-retry_status = "Retry"
error_status = "Error"
Modified: trunk/Tools/QueueStatusServer/handlers/releasepatch.py (174008 => 174009)
--- trunk/Tools/QueueStatusServer/handlers/releasepatch.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/QueueStatusServer/handlers/releasepatch.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -54,13 +54,7 @@
# Ideally we should use a transaction for the calls to
# WorkItems and ActiveWorkItems.
- # Only remove it from the queue if the last message is not a retry request.
- # Allow removing it from the queue even if there is no last_status for easier testing.
- if not last_status or not last_status.is_retry_request():
- queue.work_items().remove_work_item(attachment_id)
- RecordPatchEvent.stopped(attachment_id, queue_name)
- else:
- RecordPatchEvent.retrying(attachment_id, queue_name)
+ queue.work_items().remove_work_item(attachment_id)
+ RecordPatchEvent.stopped(attachment_id, queue_name)
- # Always release the lock on the item.
queue.active_work_items().expire_item(attachment_id)
Modified: trunk/Tools/QueueStatusServer/handlers/submittoews.py (174008 => 174009)
--- trunk/Tools/QueueStatusServer/handlers/submittoews.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/QueueStatusServer/handlers/submittoews.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -45,14 +45,11 @@
# in adding things to the commit-queue when they won't be processed by it.
assert(queue.is_ews())
latest_status = attachment.status_for_queue(queue)
- if not latest_status:
- return True
- # Only ever re-submit to the EWS if the EWS specifically requested a retry.
- # This allows us to restart the EWS feeder queue, without all r? patches
- # being retried as a result of that restart!
- # In some future version we might add a "force" button to allow the user
- # to override this restriction.
- return latest_status.is_retry_request()
+ # The feeder queue only submits each patch once normally, but it loses its memory
+ # when restarted. We do not want to re-add all patches if that happens.
+ # In the future we might add a "force" button to allow the user to retry patches
+ # that were previously submitted.
+ return not latest_status
def _add_attachment_to_ews_queues(self, attachment):
for queue in Queue.all_ews(): # all_ews() currently includes the style-queue
Modified: trunk/Tools/QueueStatusServer/loggers/recordpatchevent.py (174008 => 174009)
--- trunk/Tools/QueueStatusServer/loggers/recordpatchevent.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/QueueStatusServer/loggers/recordpatchevent.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -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
@@ -41,32 +42,25 @@
queue_log.put()
@classmethod
- def retrying(cls, attachment_id, queue_name, bot_id=None):
+ def started(cls, attachment_id, queue_name, bot_id=None):
patch_log = PatchLog.lookup_if_exists(attachment_id, queue_name)
if not patch_log:
- WarningLog.record("patchlog missing", "In retrying event.", attachment_id, queue_name, bot_id)
+ WarningLog.record("patchlog missing", "In started event.", attachment_id, queue_name, bot_id)
return
if bot_id:
patch_log.bot_id = bot_id
- patch_log.retry_count += 1
- patch_log.put()
- queue_log = QueueLog.get_current(queue_name, queue_log_duration)
- queue_log.patch_retry_count += 1
- queue_log.put()
+ # An existing wait_duration implies the patch had been started previously and is
+ # being picked up again because it had expired, or was released.
+ if patch_log.wait_duration:
+ patch_log.retry_count += 1
+ patch_log.put()
- @classmethod
- def started(cls, attachment_id, queue_name, bot_id=None):
- patch_log = PatchLog.lookup_if_exists(attachment_id, queue_name)
- if not patch_log:
- WarningLog.record("patchlog missing", "In started event.", attachment_id, queue_name, bot_id)
- return
-
- # An existing wait_duration implies the patch had been started previously and is being picked up again because it had expired.
- if not patch_log.wait_duration:
- if bot_id:
- patch_log.bot_id = bot_id
+ queue_log = QueueLog.get_current(queue_name, queue_log_duration)
+ queue_log.patch_retry_count += 1
+ queue_log.put()
+ else:
patch_log.calculate_wait_duration()
patch_log.put()
Modified: trunk/Tools/QueueStatusServer/model/queuestatus.py (174008 => 174009)
--- trunk/Tools/QueueStatusServer/model/queuestatus.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/QueueStatusServer/model/queuestatus.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -40,6 +40,3 @@
message = db.StringProperty(multiline=True)
date = db.DateTimeProperty(auto_now_add=True)
results_file = db.BlobProperty()
-
- def is_retry_request(self):
- return self.message == messages.retry_status
Modified: trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py (174008 => 174009)
--- trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -52,6 +52,9 @@
def release_work_item(self, queue_name, patch):
_log.info("MOCK: release_work_item: %s %s" % (queue_name, patch.id()))
+ 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):
self._work_items = work_items
_log.info("MOCK: update_work_items: %s %s" % (queue_name, work_items))
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (174008 => 174009)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -65,7 +65,6 @@
_pass_status = "Pass"
_fail_status = "Fail"
- _retry_status = "Retry"
_error_status = "Error"
def __init__(self, options=None): # Default values should never be collections (like []) as default values are shared between invocations
@@ -239,10 +238,6 @@
self._update_status(self._fail_status, patch)
self._release_work_item(patch)
- def _did_retry(self, patch):
- self._update_status(self._retry_status, patch)
- self._release_work_item(patch)
-
def _did_error(self, patch, reason):
message = "%s: %s" % (self._error_status, reason)
self._update_status(message, patch)
@@ -325,7 +320,8 @@
if task.run():
self._did_pass(patch)
return True
- self._did_retry(patch)
+ self._unlock_patch(patch)
+ return False
except ScriptError, e:
validator = CommitterValidator(self._tool)
validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task, patch, e))
@@ -333,6 +329,7 @@
if results_archive:
self._upload_results_archive_for_patch(patch, results_archive)
self._did_fail(patch)
+ return False
def _failing_tests_message(self, task, patch):
results = task.results_from_patch_test_run(patch)
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (174008 => 174009)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2014-09-26 16:58:41 UTC (rev 174009)
@@ -405,8 +405,7 @@
MOCK: update_status: commit-queue Built patch
Running: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --port=mac
MOCK: update_status: commit-queue Passed tests
-MOCK: update_status: commit-queue Retry
-MOCK: release_work_item: commit-queue 10000
+MOCK: release_lock: commit-queue 10000
"""
self.maxDiff = None
OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_logs=expected_logs)
