Sort issues by their age in the commit queue.

pending_manager.py

 # coding=utf8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Commit queue manager class.
 
 Security implications:
 
 The following hypothesis are made:
 - Commit queue:
   - Impersonate the same svn credentials that the patchset owner.
   - Can't impersonate a non committer.
   - SVN will check the committer write access.
 """
 
+import datetime
 import errno
 import logging
 import os
 import socket
 import ssl
 import time
 import traceback
 import urllib2
 
 import find_depot_tools  # pylint: disable=W0611
@@ skipping 14 matching lines @@
   issue = int
   patchset = int
   description = unicode
   files = list
   # Only a cache, these values can be regenerated.
   owner = unicode
   reviewers = list
   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)
 
   def __init__(self, **kwargs):
     super(PendingCommit, self).__init__(**kwargs)
     for message in self.messages:
       # Save storage, no verifier really need 'text', just 'approval'.
       if 'text' in message:
         del message['text']
 
@@ skipping 50 matching lines @@
   """
   pending_commits = dict
 
   def add(self, item):
     self.pending_commits[str(item.issue)] = item
 
   def get(self, key):
     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)
 
 
 class PendingManager(object):
   """Fetch new issues from rietveld, pass the issues through all of verifiers
   and then commit the patches with checkout.
   """
   FAILED_NO_MESSAGE = (
@@ skipping 24 matching lines @@
                            patch.
       verifiers: Verifiers object run after applying the patch.
     """
     if not(len(pre_patch_verifiers) or len(verifiers)):
       raise ValueError('at least one verifier should be defined (in project %s)'
                        % project_name)
 
     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
     # pace (burstiness) of commits.
     self.recent_commit_timestamps = []
     # Assert names are unique.
     names = [x.name for x in pre_patch_verifiers + verifiers]
     assert len(names) == len(set(names))
     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.
 
     Calls _new_pending_commit() on all new review found.
     """
     try:
       new_issues = self.context.rietveld.get_pending_issues()
     except urllib2.URLError as e:
       if 'timed out' in e.reason:
         # Handle timeouts gracefully. Log them and pretend there are no
@@ skipping 12 matching lines @@
         logging.info('Flushing issue %d' % pending.issue)
         pending.get_state = lambda: base.IGNORED
         self._discard_pending(pending, 'CQ bit was unchecked on CL. Ignoring.')
 
     # Find new issues.
     for issue_id in new_issues:
       if str(issue_id) not in self.queue.pending_commits:
         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.
             logging.warning('%s while accessing %s. Ignoring error.' % (
                             str(e), e.url))
             continue
           raise
         except urllib2.URLError as e:
           # Temporary AppEngine hiccup. Just log it and continue.
           if 'timed out' in e.reason:
@@ skipping 23 matching lines @@
         if issue_data['patchsets'] and issue_data['commit']:
           logging.info('Found new issue %d' % issue_id)
           self.queue.add(
               PendingCommit(
                   issue=issue_id,
                   owner=issue_data['owner_email'],
                   reviewers=issue_data['reviewers'],
                   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."""
     expected = set(i.name for i in self.all_verifiers)
     for pending in self.queue.iterate():
       try:
         # Take in account the case where a verifier was removed.
         done = set(pending.verifications.keys())
         missing = expected - done
         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
         if not message:
           message = 'process_new_pending_commit: ' + self.FAILED_NO_MESSAGE
         self._discard_pending(e.pending, message)
 
   def update_status(self):
     """Updates the status for each pending commit verifier."""
     why_nots = dict((p.issue, p.why_not()) for p in self.queue.iterate())
@@ skipping 293 matching lines @@
     """Loads the commit queue state from a JSON file."""
     self.queue = model.load_from_json_file(filename)
 
   def save(self, filename):
     """Save the commit queue state in a simple JSON file."""
     model.save_to_json_file(filename, self.queue)
 
   def close(self):
     """Close all the active pending manager items."""
     self.context.status.close()