Add presubmit check to verify issue is not closed.

rietveld.py

 # coding: utf-8
 # 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.
 """Defines class Rietveld to easily access a rietveld instance.
 
 Security implications:
 
 The following hypothesis are made:
 - Rietveld enforces:
@@ skipping 59 matching lines @@
 
   def get_pending_issues(self):
     """Returns an array of dict of all the pending issues on the server."""
     # TODO: Convert this to use Rietveld::search(), defined below.
     return json.loads(
         self.get('/search?format=json&commit=2&closed=3&'
                  'keys_only=True&limit=1000&order=__key__'))['results']
 
   def close_issue(self, issue):
     """Closes the Rietveld issue for this changelist."""
-    logging.info('closing issue %d' % issue)
-    self.post("/%d/close" % issue, [('xsrf_token', self.xsrf_token())])
+    logging.info('closing issue %s' % issue)

[M-A Ruel, 2012/11/28 02:43:49]

Please don't change these and revert everything in this file.

[Isaac (away), 2012/11/28 22:28:17]

OK...  I thought you asked for that earlier when you said rietveld.py should be
able to take an issue number as a string?

+    self.post("/%s/close" % issue, [('xsrf_token', self.xsrf_token())])
 
   def get_description(self, issue):
     """Returns the issue's description."""
-    return self.get('/%d/description' % issue)
+    return self.get('/%s/description' % issue)
 
   def get_issue_properties(self, issue, messages):
     """Returns all the issue's metadata as a dictionary."""
-    url = '/api/%d' % issue
+    url = '/api/%s' % issue
     if messages:
       url += '?messages=true'
     return json.loads(self.get(url))
 
   def get_patchset_properties(self, issue, patchset):
     """Returns the patchset properties."""
-    url = '/api/%d/%d' % (issue, patchset)
+    url = '/api/%s/%s' % (issue, patchset)
     return json.loads(self.get(url))
 
   def get_file_content(self, issue, patchset, item):
     """Returns the content of a new file.
 
     Throws HTTP 302 exception if the file doesn't exist or is not a binary file.
     """
     # content = 0 is the old file, 1 is the new file.
     content = 1
-    url = '/%d/binary/%d/%d/%d' % (issue, patchset, item, content)
+    url = '/%s/binary/%s/%d/%d' % (issue, patchset, item, content)
     return self.get(url)
 
   def get_file_diff(self, issue, patchset, item):
     """Returns the diff of the file.
 
     Returns a useless diff for binary files.
     """
-    url = '/download/issue%d_%d_%d.diff' % (issue, patchset, item)
+    url = '/download/issue%s_%s_%d.diff' % (issue, patchset, item)
     return self.get(url)
 
   def get_patch(self, issue, patchset):
     """Returns a PatchSet object containing the details to apply this patch."""
     props = self.get_patchset_properties(issue, patchset) or {}
     out = []
     for filename, state in props.get('files', {}).iteritems():
       logging.debug('%s' % filename)
       # If not status, just assume it's a 'M'. Rietveld often gets it wrong and
       # just has status: null. Oh well.
@@ skipping 103 matching lines @@
         content = rietveld_svn_props.pop(0)
         match2 = re.match(r'^   \+ (.*)$', content)
         if not match2:
           raise patch.UnsupportedPatchFormat(
               filename, 'Unsupported svn property format.')
         svn_props.append((match.group(2), match2.group(1)))
     return svn_props
 
   def update_description(self, issue, description):
     """Sets the description for an issue on Rietveld."""
-    logging.info('new description for issue %d' % issue)
-    self.post('/%d/description' % issue, [
+    logging.info('new description for issue %s' % issue)
+    self.post('/%s/description' % issue, [
         ('description', description),
         ('xsrf_token', self.xsrf_token())])
 
   def add_comment(self, issue, message, add_as_reviewer=False):
     max_message = 10000
     tail = '…\n(message too large)'
     if len(message) > max_message:
       message = message[:max_message-len(tail)] + tail
-    logging.info('issue %d; comment: %s' % (issue, message))
-    return self.post('/%d/publish' % issue, [
+    logging.info('issue %s; comment: %s' % (issue, message))
+    return self.post('/%s/publish' % issue, [
         ('xsrf_token', self.xsrf_token()),
         ('message', message),
         ('message_only', 'True'),
         ('add_as_reviewer', str(bool(add_as_reviewer))),
         ('send_mail', 'True'),
         ('no_redirect', 'True')])
 
   def set_flag(self, issue, patchset, flag, value):
-    return self.post('/%d/edit_flags' % issue, [
+    return self.post('/%s/edit_flags' % issue, [
         ('last_patchset', str(patchset)),
         ('xsrf_token', self.xsrf_token()),
         (flag, value)])
 
   def search(
       self,
       owner=None, reviewer=None,
       base=None,
       closed=None, private=None, commit=None,
       created_before=None, created_after=None,
@@ skipping 58 matching lines @@
     Returns the keys of the new TryJobResult entites.
     """
     params = [
       ('reason', reason),
       ('clobber', 'True' if clobber else 'False'),
       ('builders', json.dumps(builders_and_tests)),
       ('xsrf_token', self.xsrf_token()),
     ]
     if revision:
       params.append(('revision', revision))
-    return self.post('/%d/try/%d' % (issue, patchset), params)
+    return self.post('/%s/try/%s' % (issue, patchset), params)
 
   def get_pending_try_jobs(self, cursor=None, limit=100):
     """Retrieves the try job requests in pending state.
 
     Returns a tuple of the list of try jobs and the cursor for the next request.
     """
     url = '/get_pending_try_patchsets?limit=%d' % limit
     extra = ('&cursor=' + cursor) if cursor else ''
     data = json.loads(self.get(url + extra))
     return data['jobs'], data['cursor']
@@ skipping 40 matching lines @@
           if not 'Name or service not known' in e.reason:
             # Usually internal GAE flakiness.
             raise
         # If reaching this line, loop again. Uses a small backoff.
         time.sleep(1+maxtries*2)
     finally:
       upload.ErrorExit = old_error_exit
 
   # DEPRECATED.
   Send = get