Remove CL upload logic from Perf Try Job workflow.

tools/perf/core/trybot_command.py

Index: tools/perf/core/trybot_command.py
diff --git a/tools/perf/core/trybot_command.py b/tools/perf/core/trybot_command.py
index 86fe1cf4bd1fbc4987fc3c057a97bbab371d3881..88c335e8a25f2ae11451082986f06a9562e2318d 100644
--- a/tools/perf/core/trybot_command.py
+++ b/tools/perf/core/trybot_command.py
@@ -7,8 +7,8 @@ import json
 import logging
 import os
 import platform
-import re
 import subprocess
+import tempfile
 import urllib2
 
 
@@ -446,13 +446,8 @@ E.g.,
     output = RunGit(['diff-index', 'HEAD'])
     if output:
       raise TrybotError(
-          'Cannot send a try job with a dirty tree. Please commit '
-          'your changes locally first in %s repository.' % repo_path)
-
-    # Make sure the tree does have local commits.
-    output = RunGit(['footers', 'HEAD'])
-    if output:
-      raise TrybotError('No local changes found in %s repository.' % repo_path)
+          'Cannot send a try job with a dirty tree.\nPlease commit locally and '
+          'upload your changes to rietveld in %s repository.' % repo_path)
 
     return (repo_name, branch_name)
 
@@ -501,6 +496,35 @@ E.g.,
     raise TrybotError('URL %s on remote %s is not recognized on branch.'% (
         cur_remote_url, cur_remote))
 
+  def _GetChangeList(self):
+    """Gets the codereview URL for the current changes."""
+    temp_file = None
+    json_output = None
+    try:
+      fd, temp_file = tempfile.mkstemp(suffix='.json', prefix='cl')

[Michael Achenbach, 2016/10/08 09:43:29]

nit: Maybe add a slightly longer and more verbose prefix? I see you delete it
below, but if tempfiles are (for whatever reason) not deleted, it would be way
easier to reason where they come from.

[prasadv1, 2016/10/10 18:22:56]

Done.

+      os.close(fd)
+      RunGit(['cl', 'issue', '--json', temp_file],
+              'Failed to run "git cl issue" command.')
+      try:
+        with open(temp_file, 'r') as f:
+          json_output = json.load(f)
+      except (IOError, ValueError):

[Michael Achenbach, 2016/10/08 09:43:29]

Catching this here and setting json_output to None will make json_output.get
fail below. And it would be obscure what the original error was.

Maybe just don't catch the exceptions or recover from them in a more robust way?

[prasadv1, 2016/10/10 18:22:56]

Done.

+        json_output = None
+    finally:
+      try:
+        if temp_file:
+          os.remove(temp_file)
+      except OSError:
+        pass
+
+    # Make sure the local commits are uploaded to rietveld.
+    if not json_output.get('issue'):
+      raise TrybotError(
+          'PLEASE NOTE: The workflow for Perf Try jobs is changed. '
+          'In order to run the perf try job, you must first upload your '
+          'changes to rietveld.')
+    return json_output.get('issue_url')
+
   def _AttemptTryjob(self, options, extra_args):
     """Attempts to run a tryjob from a repo directory.
 
@@ -533,10 +557,8 @@ E.g.,
               branch_name, repo_info.get('url'))
         deps_override = {repo_info.get('src'): options.deps_revision}
 
-      arguments = [options.benchmark_name] + extra_args
-
-      rietveld_url = self._UploadPatchToRietveld(repo_name, options)
-      print ('\nUploaded try job to rietveld.\nview progress here %s.'
+      rietveld_url = self._GetChangeList()
+      print ('\nRunning try job....\nview progress here %s.'
              '\n\tRepo Name: %s\n\tPath: %s\n\tBranch: %s' % (
                  rietveld_url, repo_name, repo_path, branch_name))
 
@@ -545,6 +567,7 @@ E.g.,
           logging.warning('No builder is found for %s', bot_platform)
           continue
         try:
+          arguments = [options.benchmark_name] + extra_args
           self._RunTryJob(bot_platform, arguments, deps_override)
         # Even if git cl try throws TrybotError exception for any platform,
         # keep sending try jobs to other platforms.
@@ -558,20 +581,6 @@ E.g.,
       os.chdir(original_workdir)
     return 0
 
-  def _UploadPatchToRietveld(self, repo_name, options):
-    """Uploads the patch to rietveld and returns rietveld URL."""
-    output = RunGit(['cl', 'upload', '-f', '--bypass-hooks', '-m',
-                     ('CL for %s perf tryjob to run %s benchmark '
-                      'on %s platform(s)' % (
-                          repo_name, options.benchmark_name, options.trybot))],
-                    'Could not upload to rietveld for %s' % repo_name)
-
-    match = re.search(r'https://codereview.chromium.org/[\d]+', output)
-    if not match:
-      raise TrybotError('Could not upload CL to rietveld for %s! Output %s' %
-                        (repo_name, output))
-    return match.group(0)
-
   def _RunTryJob(self, bot_platform, arguments, deps_override):
     """Executes perf try job with benchmark test properties.