git cl try: make code less Rietveld-specific.

git_cl.py

 #!/usr/bin/env python
 # 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.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld and Gerrit."""
 
 from __future__ import print_function
@@ skipping 300 matching lines @@
     if response.status < 500 or try_count >= 2:
       raise httplib2.HttpLib2Error(content)
 
     # status >= 500 means transient failures.
     logging.debug('Transient errors when %s. Will retry.', operation_name)
     time.sleep(0.5 + 1.5*try_count)
     try_count += 1
   assert False, 'unreachable'
 
 
-def trigger_try_jobs(auth_config, changelist, options, masters, category):
-  rietveld_url = settings.GetDefaultServerUrl()
-  rietveld_host = urlparse.urlparse(rietveld_url).hostname
-  authenticator = auth.get_authenticator_for_host(rietveld_host, auth_config)
+def _trigger_try_jobs(auth_config, changelist, masters, options,
+                      category='git_cl_try', patchset=None):
+  assert changelist.GetIssue(), 'CL must be uploaded first'
+  codereview_url = changelist.GetCodereviewServer()
+  assert codereview_url, 'CL must be uploaded first'
+  patchset = patchset or changelist.GetMostRecentPatchset()
+  assert patchset, 'CL must be uploaded first'
+
+  codereview_host = urlparse.urlparse(codereview_url).hostname
+  authenticator = auth.get_authenticator_for_host(codereview_host, auth_config)
   http = authenticator.authorize(httplib2.Http())
   http.force_exception_to_status_code = True
-  issue_props = changelist.GetIssueProperties()
-  issue = changelist.GetIssue()
-  patchset = changelist.GetMostRecentPatchset()
-  properties = _get_properties_from_options(options)
+
+  # TODO(tandrii): consider caching Gerrit CL details just like
+  # _RietveldChangelistImpl does, then caching values in these two variables
+  # won't be necessary.
+  owner_email = changelist.GetIssueOwner()
+  project = changelist.GetIssueProject()
 
   buildbucket_put_url = (
       'https://{hostname}/_ah/api/buildbucket/v1/builds/batch'.format(
           hostname=options.buildbucket_host))
-  buildset = 'patch/rietveld/{hostname}/{issue}/{patch}'.format(
-      hostname=rietveld_host,
-      issue=issue,
+  buildset = 'patch/{codereview}/{hostname}/{issue}/{patch}'.format(
+      codereview='gerrit' if changelist.IsGerrit() else 'rietveld',
+      hostname=codereview_host,
+      issue=changelist.GetIssue(),
       patch=patchset)
+  extra_properties = _get_properties_from_options(options)
 
   batch_req_body = {'builds': []}
   print_text = []
   print_text.append('Tried jobs on:')
   for master, builders_and_tests in sorted(masters.iteritems()):
     print_text.append('Master: %s' % master)
     bucket = _prefix_master(master)
     for builder, tests in sorted(builders_and_tests.iteritems()):
       print_text.append('  %s: %s' % (builder, tests))
       parameters = {
           'builder_name': builder,
           'changes': [{
-              'author': {'email': issue_props['owner_email']},
+              'author': {'email': owner_email},
               'revision': options.revision,
           }],
           'properties': {
               'category': category,
-              'issue': issue,
+              'issue': changelist.GetIssue(),
               'master': master,
-              'patch_project': issue_props['project'],
+              'patch_project': project,
               'patch_storage': 'rietveld',
               'patchset': patchset,
               'reason': options.name,
-              'rietveld': rietveld_url,
+              'rietveld': codereview_url,
           },
       }
       if 'presubmit' in builder.lower():
         parameters['properties']['dry_run'] = 'true'
       if tests:
         parameters['properties']['testfilter'] = tests
-      if properties:
-        parameters['properties'].update(properties)
+      if extra_properties:
+        parameters['properties'].update(extra_properties)
       if options.clobber:
         parameters['properties']['clobber'] = True
       batch_req_body['builds'].append(
           {
               'bucket': bucket,
               'parameters_json': json.dumps(parameters),
               'client_operation_id': str(uuid.uuid4()),
               'tags': ['builder:%s' % builder,
                        'buildset:%s' % buildset,
                        'master:%s' % master,
@@ skipping 1160 matching lines @@
 
   def SetCQState(self, new_state):
     """Update the CQ state for latest patchset.
 
     Issue must have been already uploaded and known.
     """
     assert new_state in _CQState.ALL_STATES
     assert self.GetIssue()
     return self._codereview_impl.SetCQState(new_state)
 
-  def CannotTriggerTryJobReason(self):
-    """Returns reason (str) if unable trigger tryjobs on this CL or None."""
-    return self._codereview_impl.CannotTriggerTryJobReason()
-
   # Forward methods to codereview specific implementation.
 
   def CloseIssue(self):
     return self._codereview_impl.CloseIssue()
 
   def GetStatus(self):
     return self._codereview_impl.GetStatus()
 
   def GetCodereviewServer(self):
     return self._codereview_impl.GetCodereviewServer()
 
+  def GetIssueOwner(self):
+    """Get owner from codereview, which may differ from this checkout."""
+    return self._codereview_impl.GetIssueOwner()
+
+  def GetIssueProject(self):
+    """Get project from codereview, which may differ from what this
+    checkout's codereview.settings or gerrit project URL say.
+    """
+    return self._codereview_impl.GetIssueProject()
+
   def GetApprovingReviewers(self):
     return self._codereview_impl.GetApprovingReviewers()
 
   def GetMostRecentPatchset(self):
     return self._codereview_impl.GetMostRecentPatchset()
 
+  def CannotTriggerTryJobReason(self):
+    """Returns reason (str) if unable trigger tryjobs on this CL or None."""
+    return self._codereview_impl.CannotTriggerTryJobReason()
+
   def __getattr__(self, attr):
     # This is because lots of untested code accesses Rietveld-specific stuff
     # directly, and it's hard to fix for sure. So, just let it work, and fix
     # on a case by case basis.
     # Note that child method defines __getattr__ as well, and forwards it here,
     # because _RietveldChangelistImpl is not cleaned up yet, and given
     # deprecation of Rietveld, it should probably be just removed.
     # Until that time, avoid infinite recursion by bypassing __getattr__
     # of implementation class.
     return self._codereview_impl.__getattribute__(attr)
@@ skipping 109 matching lines @@
     """Update the CQ state for latest patchset.
 
     Issue must have been already uploaded and known.
     """
     raise NotImplementedError()
 
   def CannotTriggerTryJobReason(self):
     """Returns reason (str) if unable trigger tryjobs on this CL or None."""
     raise NotImplementedError()
 
+  def GetIssueOwner(self):
+    raise NotImplementedError()
+
+  def GetIssueProject(self):
+    raise NotImplementedError()
+
 
 class _RietveldChangelistImpl(_ChangelistCodereviewBase):
   def __init__(self, changelist, auth_config=None, rietveld_server=None):
     super(_RietveldChangelistImpl, self).__init__(changelist)
     assert settings, 'must be initialized in _ChangelistCodereviewBase'
     if not rietveld_server:
       settings.GetDefaultServerUrl()
 
     self._rietveld_server = rietveld_server
     self._auth_config = auth_config
@@ skipping 65 matching lines @@
       return 'Rietveld doesn\'t know about your issue %s' % self.GetIssue()
     if props.get('closed'):
       return 'CL %s is closed' % self.GetIssue()
     if props.get('private'):
       return 'CL %s is private' % self.GetIssue()
     return None
 
   def GetApprovingReviewers(self):
     return get_approving_reviewers(self.GetIssueProperties())
 
+  def GetIssueOwner(self):
+    return (self.GetIssueProperties() or {}).get('owner_email')
+
+  def GetIssueProject(self):
+    return (self.GetIssueProperties() or {}).get('project')
+
   def AddComment(self, message):
     return self.RpcServer().add_comment(self.GetIssue(), message)
 
   def GetStatus(self):
     """Apply a rough heuristic to give a simple summary of an issue's review
     or CQ status, assuming adherence to a common workflow.
 
     Returns None if no issue for this branch, or one of the following keywords:
       * 'error'   - error from review tool (including deleted issues)
       * 'unsent'  - not sent for review
@@ skipping 935 matching lines @@
         _CQState.DRY_RUN: 1,
         _CQState.COMMIT : 2,
     }
     gerrit_util.SetReview(self._GetGerritHost(), self.GetIssue(),
                           labels={'Commit-Queue': vote_map[new_state]})
 
   def CannotTriggerTryJobReason(self):
     # TODO(tandrii): implement for Gerrit.
     raise NotImplementedError()
 
+  def GetIssueOwner(self):
+    # TODO(tandrii): implement for Gerrit.
+    raise NotImplementedError()
+
+  def GetIssueProject(self):
+    # TODO(tandrii): implement for Gerrit.
+    raise NotImplementedError()
+
 
 _CODEREVIEW_IMPLEMENTATIONS = {
   'rietveld': _RietveldChangelistImpl,
   'gerrit': _GerritChangelistImpl,
 }
 
 
 def _add_codereview_issue_select_options(parser, extra=""):
   _add_codereview_select_options(parser)
 
@@ skipping 1959 matching lines @@
 
   cl = Changelist(auth_config=auth_config)
   if not cl.GetIssue():
     parser.error('Need to upload first')
 
   if cl.IsGerrit():
     parser.error(
         'Not yet supported for Gerrit (http://crbug.com/599931).\n'
         'If your project has Commit Queue, dry run is a workaround:\n'
         '  git cl set-commit --dry-run')
-  # Code below assumes Rietveld issue.
-  # TODO(tandrii): actually implement for Gerrit http://crbug.com/599931.
 
   error_message = cl.CannotTriggerTryJobReason()
   if error_message:
     parser.error('Can\'t trigger try jobs: %s')
 
   if not options.name:
     options.name = cl.GetBranch()
 
   if options.bot and not options.master:
     options.master, err_msg = GetBuilderMaster(options.bot)
@@ skipping 74 matching lines @@
             'Consider specifying which bots to trigger manually '
             'or asking your project owners for permissions '
             'or contacting Chrome Infrastructure team at '
             'https://www.chromium.org/infra\n\n')
       # Still raise exception so that stack trace is printed.
       raise
 
   for builders in masters.itervalues():
     if any('triggered' in b for b in builders):
       print('ERROR You are trying to send a job to a triggered bot. This type '
-            'of bot requires an\ninitial job from a parent (usually a builder).'
-            '  Instead send your job to the parent.\n'
+            'of bot requires an initial job from a parent (usually a builder). '
+            'Instead send your job to the parent.\n'
             'Bot list: %s' % builders, file=sys.stderr)
       return 1
 
   patchset = cl.GetMostRecentPatchset()
-  if patchset and patchset != cl.GetPatchset():
-    print(
-        '\nWARNING Mismatch between local config and server. Did a previous '
-        'upload fail?\ngit-cl try always uses latest patchset from rietveld. '
-        'Continuing using\npatchset %s.\n' % patchset)
+  if patchset != cl.GetPatchset():
+    print('Warning: Codereview server has newer patchsets (%s) than most '
+          'recent upload from local checkout (%s). Did a previous upload '
+          'fail?\n'
+          'By default, git cl try uses the latest patchset from '
+          'codereview, continuing to use patchset %s.\n' %
+          (patchset, cl.GetPatchset(), patchset))
   try:
-    trigger_try_jobs(auth_config, cl, options, masters, 'git_cl_try')
+    _trigger_try_jobs(auth_config, cl, masters, options, 'git_cl_try',
+                     patchset)
   except BuildbucketResponseException as ex:
     print('ERROR: %s' % ex)
     return 1
-  except Exception as e:
-    stacktrace = (''.join(traceback.format_stack()) + traceback.format_exc())
-    print('ERROR: Exception when trying to trigger try jobs: %s\n%s' %
-          (e, stacktrace))
-    return 1
   return 0
 
 
 def CMDtry_results(parser, args):
   """Prints info about try jobs associated with current CL."""
   group = optparse.OptionGroup(parser, 'Try job results options')
   group.add_option(
       '-p', '--patchset', type=int, help='patchset number if not current.')
   group.add_option(
       '--print-master', action='store_true', help='print master name as well.')
@@ skipping 22 matching lines @@
     if not patchset:
       parser.error('Codereview doesn\'t know about issue %s. '
                    'No access to issue or wrong issue number?\n'
                    'Either upload first, or pass --patchset explicitely' %
                    cl.GetIssue())
 
     if patchset != cl.GetPatchset():
       print('Warning: Codereview server has newer patchsets (%s) than most '
             'recent upload from local checkout (%s). Did a previous upload '
             'fail?\n'
-            'By default, git cl try uses latest patchset from codereview, '
-            'continuing to use patchset %s.\n' %
+            'By default, git cl try-results uses the latest patchset from '
+            'codereview, continuing to use patchset %s.\n' %
             (patchset, cl.GetPatchset(), patchset))
   try:
     jobs = fetch_try_jobs(auth_config, cl, options.buildbucket_host, patchset)
   except BuildbucketResponseException as ex:
     print('Buildbucket error: %s' % ex)
     return 1
   if options.json:
     write_try_results_json(options.json, jobs)
   else:
     print_try_jobs(options, jobs)
@@ skipping 438 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)