<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[174009] trunk/Tools</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/174009">174009</a></dd>
<dt>Author</dt> <dd>ap@apple.com</dd>
<dt>Date</dt> <dd>2014-09-26 09:58:41 -0700 (Fri, 26 Sep 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsQueueStatusServerappyaml">trunk/Tools/QueueStatusServer/app.yaml</a></li>
<li><a href="#trunkToolsQueueStatusServerconfigmessagespy">trunk/Tools/QueueStatusServer/config/messages.py</a></li>
<li><a href="#trunkToolsQueueStatusServerhandlersreleasepatchpy">trunk/Tools/QueueStatusServer/handlers/releasepatch.py</a></li>
<li><a href="#trunkToolsQueueStatusServerhandlerssubmittoewspy">trunk/Tools/QueueStatusServer/handlers/submittoews.py</a></li>
<li><a href="#trunkToolsQueueStatusServerloggersrecordpatcheventpy">trunk/Tools/QueueStatusServer/loggers/recordpatchevent.py</a></li>
<li><a href="#trunkToolsQueueStatusServermodelqueuestatuspy">trunk/Tools/QueueStatusServer/model/queuestatus.py</a></li>
<li><a href="#trunkToolsScriptswebkitpycommonnetstatusserver_mockpy">trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py</a></li>
<li><a href="#trunkToolsScriptswebkitpytoolcommandsqueuespy">trunk/Tools/Scripts/webkitpy/tool/commands/queues.py</a></li>
<li><a href="#trunkToolsScriptswebkitpytoolcommandsqueues_unittestpy">trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2014-09-26 16:57:45 UTC (rev 174008)
+++ trunk/Tools/ChangeLog 2014-09-26 16:58:41 UTC (rev 174009)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2014-09-26 Alexey Proskuryakov <ap@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2014-09-26 Csaba Osztrogonác <ossy@webkit.org>
</span><span class="cx">
</span><span class="cx"> [EFL] Fix the gst-libav build on ARM Thumb2
</span></span></pre></div>
<a id="trunkToolsQueueStatusServerappyaml"></a>
<div class="modfile"><h4>Modified: trunk/Tools/QueueStatusServer/app.yaml (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> application: webkit-queues
</span><del>-version: 173979 # Bugzilla bug ID of last major change
</del><ins>+version: 174009 # Bugzilla bug ID of last major change
</ins><span class="cx"> runtime: python
</span><span class="cx"> api_version: 1
</span><span class="cx">
</span></span></pre></div>
<a id="trunkToolsQueueStatusServerconfigmessagespy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/QueueStatusServer/config/messages.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -29,5 +29,4 @@
</span><span class="cx"> # These must be in sync with webkit-patch's AbstractQueue.
</span><span class="cx"> pass_status = "Pass"
</span><span class="cx"> fail_status = "Fail"
</span><del>-retry_status = "Retry"
</del><span class="cx"> error_status = "Error"
</span></span></pre></div>
<a id="trunkToolsQueueStatusServerhandlersreleasepatchpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/QueueStatusServer/handlers/releasepatch.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -54,13 +54,7 @@
</span><span class="cx"> # Ideally we should use a transaction for the calls to
</span><span class="cx"> # WorkItems and ActiveWorkItems.
</span><span class="cx">
</span><del>- # 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)
</del><ins>+ queue.work_items().remove_work_item(attachment_id)
+ RecordPatchEvent.stopped(attachment_id, queue_name)
</ins><span class="cx">
</span><del>- # Always release the lock on the item.
</del><span class="cx"> queue.active_work_items().expire_item(attachment_id)
</span></span></pre></div>
<a id="trunkToolsQueueStatusServerhandlerssubmittoewspy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/QueueStatusServer/handlers/submittoews.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -45,14 +45,11 @@
</span><span class="cx"> # in adding things to the commit-queue when they won't be processed by it.
</span><span class="cx"> assert(queue.is_ews())
</span><span class="cx"> latest_status = attachment.status_for_queue(queue)
</span><del>- 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()
</del><ins>+ # 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
</ins><span class="cx">
</span><span class="cx"> def _add_attachment_to_ews_queues(self, attachment):
</span><span class="cx"> for queue in Queue.all_ews(): # all_ews() currently includes the style-queue
</span></span></pre></div>
<a id="trunkToolsQueueStatusServerloggersrecordpatcheventpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/QueueStatusServer/loggers/recordpatchevent.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,4 +1,5 @@
</span><span class="cx"> # Copyright (C) 2013 Google Inc. All rights reserved.
</span><ins>+# Copyright (C) 2014 Apple Inc. All rights reserved.
</ins><span class="cx"> #
</span><span class="cx"> # Redistribution and use in source and binary forms, with or without
</span><span class="cx"> # modification, are permitted provided that the following conditions are
</span><span class="lines">@@ -41,32 +42,25 @@
</span><span class="cx"> queue_log.put()
</span><span class="cx">
</span><span class="cx"> @classmethod
</span><del>- def retrying(cls, attachment_id, queue_name, bot_id=None):
</del><ins>+ def started(cls, attachment_id, queue_name, bot_id=None):
</ins><span class="cx"> patch_log = PatchLog.lookup_if_exists(attachment_id, queue_name)
</span><span class="cx"> if not patch_log:
</span><del>- WarningLog.record("patchlog missing", "In retrying event.", attachment_id, queue_name, bot_id)
</del><ins>+ WarningLog.record("patchlog missing", "In started event.", attachment_id, queue_name, bot_id)
</ins><span class="cx"> return
</span><span class="cx">
</span><span class="cx"> if bot_id:
</span><span class="cx"> patch_log.bot_id = bot_id
</span><del>- patch_log.retry_count += 1
- patch_log.put()
</del><span class="cx">
</span><del>- queue_log = QueueLog.get_current(queue_name, queue_log_duration)
- queue_log.patch_retry_count += 1
- queue_log.put()
</del><ins>+ # 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()
</ins><span class="cx">
</span><del>- @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
</del><ins>+ queue_log = QueueLog.get_current(queue_name, queue_log_duration)
+ queue_log.patch_retry_count += 1
+ queue_log.put()
+ else:
</ins><span class="cx"> patch_log.calculate_wait_duration()
</span><span class="cx"> patch_log.put()
</span><span class="cx">
</span></span></pre></div>
<a id="trunkToolsQueueStatusServermodelqueuestatuspy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/QueueStatusServer/model/queuestatus.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -40,6 +40,3 @@
</span><span class="cx"> message = db.StringProperty(multiline=True)
</span><span class="cx"> date = db.DateTimeProperty(auto_now_add=True)
</span><span class="cx"> results_file = db.BlobProperty()
</span><del>-
- def is_retry_request(self):
- return self.message == messages.retry_status
</del></span></pre></div>
<a id="trunkToolsScriptswebkitpycommonnetstatusserver_mockpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/common/net/statusserver_mock.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -52,6 +52,9 @@
</span><span class="cx"> def release_work_item(self, queue_name, patch):
</span><span class="cx"> _log.info("MOCK: release_work_item: %s %s" % (queue_name, patch.id()))
</span><span class="cx">
</span><ins>+ def release_lock(self, queue_name, patch):
+ _log.info("MOCK: release_lock: %s %s" % (queue_name, patch.id()))
+
</ins><span class="cx"> def update_work_items(self, queue_name, work_items):
</span><span class="cx"> self._work_items = work_items
</span><span class="cx"> _log.info("MOCK: update_work_items: %s %s" % (queue_name, work_items))
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpytoolcommandsqueuespy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -65,7 +65,6 @@
</span><span class="cx">
</span><span class="cx"> _pass_status = "Pass"
</span><span class="cx"> _fail_status = "Fail"
</span><del>- _retry_status = "Retry"
</del><span class="cx"> _error_status = "Error"
</span><span class="cx">
</span><span class="cx"> def __init__(self, options=None): # Default values should never be collections (like []) as default values are shared between invocations
</span><span class="lines">@@ -239,10 +238,6 @@
</span><span class="cx"> self._update_status(self._fail_status, patch)
</span><span class="cx"> self._release_work_item(patch)
</span><span class="cx">
</span><del>- def _did_retry(self, patch):
- self._update_status(self._retry_status, patch)
- self._release_work_item(patch)
-
</del><span class="cx"> def _did_error(self, patch, reason):
</span><span class="cx"> message = "%s: %s" % (self._error_status, reason)
</span><span class="cx"> self._update_status(message, patch)
</span><span class="lines">@@ -325,7 +320,8 @@
</span><span class="cx"> if task.run():
</span><span class="cx"> self._did_pass(patch)
</span><span class="cx"> return True
</span><del>- self._did_retry(patch)
</del><ins>+ self._unlock_patch(patch)
+ return False
</ins><span class="cx"> except ScriptError, e:
</span><span class="cx"> validator = CommitterValidator(self._tool)
</span><span class="cx"> validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task, patch, e))
</span><span class="lines">@@ -333,6 +329,7 @@
</span><span class="cx"> if results_archive:
</span><span class="cx"> self._upload_results_archive_for_patch(patch, results_archive)
</span><span class="cx"> self._did_fail(patch)
</span><ins>+ return False
</ins><span class="cx">
</span><span class="cx"> def _failing_tests_message(self, task, patch):
</span><span class="cx"> results = task.results_from_patch_test_run(patch)
</span></span></pre></div>
<a id="trunkToolsScriptswebkitpytoolcommandsqueues_unittestpy"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (174008 => 174009)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -405,8 +405,7 @@
</span><span class="cx"> MOCK: update_status: commit-queue Built patch
</span><span class="cx"> Running: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --port=mac
</span><span class="cx"> MOCK: update_status: commit-queue Passed tests
</span><del>-MOCK: update_status: commit-queue Retry
-MOCK: release_work_item: commit-queue 10000
</del><ins>+MOCK: release_lock: commit-queue 10000
</ins><span class="cx"> """
</span><span class="cx"> self.maxDiff = None
</span><span class="cx"> OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_logs=expected_logs)
</span></span></pre>
</div>
</div>
</body>
</html>