Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(587)

Unified Diff: pending_manager.py

Issue 145293006: Sort issues by their age in the commit queue. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/tools/commit-queue
Patch Set: Use rietveld.email instead of passing user Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698