CQ: don't treat the commit box as unchecked on network errors

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:
@@ skipping 157 matching lines @@
     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 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.
     """
-    new_issues = self._fetch_pending_issues()
+    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
+        # pending issues. We'll retry on the next iteration.
+        logging.warn('request to fetch pending issues timed out: %s' % e)
+        return
+
+      raise
 
     # If there is an issue in processed_issues that is not in new_issues,
     # discard it.
     for pending in self.queue.iterate():
       # Note that pending.issue is a int but self.queue.pending_commits keys
       # are str due to json support.
       if pending.issue not in new_issues:
         logging.info('Flushing issue %d' % pending.issue)

[Paweł Hajdan Jr., 2013/11/22 18:01:10]

Note that now when we catch an error we won't get here and effectively uncheck
the commit checkbox. Previously _fetch_pending_issues would return empty list
and trigger this code for all pending CLs. Baaad - my mistake, and this patch
fixes it.

         self.context.status.send(
             pending,
             { 'verification': 'abort',
               'payload': {
                 'output': 'CQ bit was unchecked on CL. Ignoring.' }})
         pending.get_state = lambda: base.IGNORED
         self._discard_pending(pending, None)
 
     # Find new issues.
     for issue_id in new_issues:
@@ skipping 39 matching lines @@
           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']))
 
-  def _fetch_pending_issues(self):
-    """Returns the list of issue number for reviews on Rietveld with their last
-    patchset with commit+ flag set.
-    """
-    try:
-      return 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
-        # pending issues. We'll retry on the next iteration.
-        logging.warn('request to fetch pending issues timed out: %s' % e)
-        return []
-
-      raise
-
   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
@@ skipping 290 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()