Index: pending_manager.py |
diff --git a/pending_manager.py b/pending_manager.py |
index c392a91d0941dbf27d651cbd31f41f0e58e35572..6f61d4b8641745cd1b41dce570bed1dcc5824ba9 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) |
@@ -174,6 +176,34 @@ class PendingManager(object): |
for verifier in self.pre_patch_verifiers: |
assert not isinstance(verifier, base.VerifierCheckout) |
+ def _get_user(self): |
+ """Get the CQ's rietveld user name. |
+ |
+ We need it to look for messages posted by the CQ, to figure out |
+ when the commit box was checked. TODO(sergeyberezin): when |
+ Rietveld can tell this info natively, this function will be |
+ obsolete. |
+ """ |
+ # Rietveld object or its email may be missing in some unittests. |
+ if self.context.rietveld and self.context.rietveld.email: |
+ return self.context.rietveld.email |
+ else: |
+ return 'commit-bot@chromium.org' |
+ |
+ 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._get_user() and |
+ 'CQ is trying da patch.' in m['text']): |
+ 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 +236,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 +280,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 +294,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 |