Sort issues by their age in the commit queue.

pending_manager.py

Index: pending_manager.py
diff --git a/pending_manager.py b/pending_manager.py
index c392a91d0941dbf27d651cbd31f41f0e58e35572..deddb94f9a8ff91218fad87e0e5d203d236039bf 100644
--- a/pending_manager.py
+++ b/pending_manager.py
@@ -13,6 +13,7 @@ The following hypothesis are made:
   - SVN will check the committer write access.
 """
 
+import datetime
 import errno
 import logging
 import os
@@ -47,6 +48,7 @@ class PendingCommit(base.Verified):
   base_url = unicode
   messages = list
   relpath = unicode
+  sort_key = unicode
   # Only used after a patch was committed. Keeping here for try job retries.
   revision = (None, int, unicode)
 
@@ -117,8 +119,8 @@ class PendingQueue(model.PersistentMixIn):
     return self.pending_commits[str(key)]
 
   def iterate(self):
-    """Returns the items sorted by issue id to ease testability."""
-    return sorted(self.pending_commits.itervalues(), key=lambda x: x.issue)
+    """Returns the items sorted by .sort_key to ease testability."""
+    return sorted(self.pending_commits.itervalues(), key=lambda x: x.sort_key)
 
   def remove(self, key):
     self.pending_commits.pop(str(key), None)
@@ -163,6 +165,7 @@ class PendingManager(object):
     self.context = context_obj
     self.pre_patch_verifiers = pre_patch_verifiers or []
     self.verifiers = verifiers or []
+    self.user = self.context.user
     self.all_verifiers = pre_patch_verifiers + verifiers
     self.queue = PendingQueue()
     # Keep the timestamps of the last few commits so that we can control the
@@ -174,6 +177,20 @@ class PendingManager(object):
     for verifier in self.pre_patch_verifiers:
       assert not isinstance(verifier, base.VerifierCheckout)
 
+  def _set_sort_key(self, issue_data):
+    """Compute the sorting key. Must add .sort_key to issue_data.
+
+    Previously we used issue id, but a better key is the last
+    timestamp when Commit box was checked.
+    """
+    for m in issue_data['messages']:
+      if (m['sender'] == self.user
+          and 'CQ is trying da patch.' in m['text']):

[Paweł Hajdan Jr., 2014/01/29 02:38:24]

nit: Put the "and" at the end of the previous line.

[Sergey Berezin, 2014/01/29 17:41:43]

Done.

+        issue_data['sort_key'] = m['date']
+    if 'sort_key' not in issue_data:
+      # Set a default value: the current time.
+      issue_data['sort_key'] = str(datetime.datetime.utcnow())
+
   def look_for_new_pending_commit(self):
     """Looks for new reviews on self.context.rietveld with c+ set.
 
@@ -206,6 +223,7 @@ class PendingManager(object):
         try:
           issue_data = self.context.rietveld.get_issue_properties(
               issue_id, True)
+          self._set_sort_key(issue_data)
         except urllib2.HTTPError as e:
           if e.code in (500, 502, 503):
             # Temporary AppEngine hiccup. Just log it and continue.
@@ -249,7 +267,8 @@ class PendingManager(object):
                   patchset=issue_data['patchsets'][-1],
                   base_url=issue_data['base_url'],
                   description=issue_data['description'].replace('\r', ''),
-                  messages=issue_data['messages']))
+                  messages=issue_data['messages'],
+                  sort_key=issue_data['sort_key']))
 
   def process_new_pending_commit(self):
     """Starts verification on newly found pending commits."""
@@ -262,8 +281,8 @@ class PendingManager(object):
         if (not missing or pending.get_state() != base.PROCESSING):
           continue
         logging.info(
-            'Processing issue %s (%s, %d)' % (
-                pending.issue, missing, pending.get_state()))
+            'Processing issue %s @ %s (%s, %d)' % (
+                pending.issue, pending.sort_key, missing, pending.get_state()))
         self._verify_pending(pending)
       except base.DiscardPending as e:
         message = e.status