Make bisect script to post build request to try server.

tools/post_perf_builder_job.py

+# Copyright (c) 2014 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.
+
+"""Post a try job to the Try server to produce build. It communicates
+to server by directly connecting via HTTP.

[qyearsley, 2014/03/06 23:40:45]

Any try server?

I have a general question about our infrastructure: Is it true that a "try
server" is a Buildbot builder (slave) which is always listening for requests
over HTTP?

Anyway, it would also be good to add in the file-level comment that this file
can be run independently and it can also be imported and used just for the
PostTryJob function. Maybe explain in what kinds of circumstances someone might
want to run this module independently, and give a usage example.

[prasadv, 2014/03/07 01:07:44]

The main purpose of this module is to post a request to tryserver to produce a
build for given a revision. Since the tryserver already has the capability to
hand such requests using TryJobHttp, we are going to use that feature for
producing perf build. Currently tryserver doesn't have perf builder bots to
handle this requests, infra team is going to add builder bot that are similar to
Chromium.perf builders.

On 2014/03/06 23:40:45, qyearsley wrote:

+"""
+
+import getpass
+import optparse
+import os
+import sys
+import urllib
+import urllib2
+
+
+class ServerAccessError(Exception):
+  def __str__(self):
+    return '%s\nSorry, cannot connect to server.' % self.args[0]
+
+
+def ParseArgs(options):

[qyearsley, 2014/03/06 23:40:45]

This function could be marked as private (_ParseArgs).

Also, the name of this function might be slightly confusing since there's an
options.parse_args() in Main which does something totally different.

Looks like the main purpose of this function is to convert the options object
output by parse_args into a dictionary of query parameters which will be passed
to PostTryJob.

[prasadv, 2014/03/07 01:07:44]

Done.

+  """Parse common options passed to PostTryJob."""

[qyearsley, 2014/03/06 23:40:45]

Style consistency note: In most places (including in the Google Python Style
Guide), docstrings start with a verb in third-person singular form (i.e.
"Parses" instead of "Parse", "Sends" instead of "Send").

[prasadv, 2014/03/07 01:07:44]

Done.

+  values = {'host': options.host,
+            'port': options.port,
+            'user': options.user,
+            'name': options.name
+           }
+  if options.email:
+    values['email'] = options.email
+  if options.revision:
+    values['revision'] = options.revision
+  if options.root:
+    values['root'] = options.root
+  if options.bot:
+    values['bot'] = options.bot
+  if options.patch:
+    values['patch'] = options.patch
+  return values
+
+
+def PostTryJob(url_params):
+  """Send a build request to the server using the HTTP protocol."""

[qyearsley, 2014/03/06 23:40:45]

This docstring could be more detailed:

 - Which parameters can be included in the dictionary that's passed in? Will
they all be sent in the query string in a GET request?
 - What does returning True mean? Will False ever be returned? Or will
ServerAccessError be raised in every possible situation where True isn't
returned?

Also, I think the PostTryJob function should be put at the top of the file, and
the functions that are currently called "ParseArgs" and "gen_parser" should be
put next to each other.

[prasadv, 2014/03/07 01:07:44]

Done.

+  # Parse url parameters to be sent to Try server.
+  if not url_params.get('host'):
+    raise ServerAccessError('Hostname of server to connect is missing.')
+  if not url_params.get('port'):
+    raise ServerAccessError('Port of server to connect is missing.')
+  if not url_params.get('revision'):
+    raise ServerAccessError('Missing revision details. Please specify revision'
+                            ' information.')
+  if not url_params.get('bot'):
+    raise ServerAccessError('Missing bot details. Please specify bot'
+                            ' information.')
+
+  url = 'http://%s:%s/send_try_patch' % (url_params['host'], url_params['port'])
+
+  print 'Sending by HTTP'
+  query_params = '&'.join('%s=%s' % (k, v) for k, v in url_params.iteritems())
+  print 'url: %s?%s' % (url, query_params)
+
+  connection = None
+  try:
+    print 'Opening connection...'
+    connection = urllib2.urlopen(url, urllib.urlencode(url_params))
+    print 'Done, request sent to server to produce build.'
+  except IOError, e:
+    raise ServerAccessError('%s is unaccessible. Reason: %s' % (url, e))
+  if not connection:
+    raise ServerAccessError('%s is unaccessible.' % url)
+  response = connection.read()
+  print 'Received %s from server' % response
+  if response != 'OK':
+    raise ServerAccessError('%s is unaccessible. Got:\n%s' % (url, response))
+  return True
+
+
+def gen_parser():

[qyearsley, 2014/03/06 23:40:45]

Style: Should be GenParser (or _GenParser). Maybe it could have a better name,
like "MakeOptionParser" or something similar.

[prasadv, 2014/03/07 01:07:44]

Done.

+  # Parse argv

[qyearsley, 2014/03/06 23:40:45]

Should have a docstring with a sentence saying what the function does.

[prasadv, 2014/03/07 01:07:44]

Done.

+  usage = ('%prog [options]\n'
+           'Post a build request to the try server for the given revision.\n')
+  parser = optparse.OptionParser(usage=usage)
+  parser.add_option('-H', '--host',
+                   help='Host address')
+  parser.add_option('-P', '--port', type='int',
+                   help='HTTP port')
+  parser.add_option('-u', '--user', default=getpass.getuser(),
+                    dest='user',
+                    help='Owner user name [default: %default]')
+  parser.add_option('-e', '--email',
+                    default=os.environ.get('TRYBOT_RESULTS_EMAIL_ADDRESS',
+                                           os.environ.get('EMAIL_ADDRESS')),
+                    help='Email address where to send the results. Use either '
+                         'the TRYBOT_RESULTS_EMAIL_ADDRESS environment '
+                         'variable or EMAIL_ADDRESS to set the email address '
+                         'the try bots report results to [default: %default]')
+  parser.add_option('-n', '--name',
+                    default= 'try_job_http',
+                    help='Descriptive name of the try job')
+  parser.add_option('-b', '--bot',
+                    help=('IMPORTANT: specify ONE builder per run is supported.'
+                          'Run script for each builders separately.'))
+  parser.add_option('-r', '--revision',
+                    help='Revision to use for the try job; default: the '
+                         'revision will be determined by the try server; see '
+                         'its waterfall for more info')
+  parser.add_option('--root',
+                   help='Root to use for the patch; base subdirectory for '
+                        'patch created in a subdirectory')
+  parser.add_option('--patch',
+                   help='Patch information.')
+  return parser
+

[qyearsley, 2014/03/06 23:40:45]

Style: Should have two blank lines here.

[prasadv, 2014/03/07 01:07:44]

Done.

+def Main(argv):
+  parser = gen_parser()
+  options, args = parser.parse_args()
+  if not options.host:
+    raise ServerAccessError('Please use the --host option to specify the try '
+        'server host to connect to.')
+  if not options.port:
+    raise ServerAccessError('Please use the --port option to specify the try '
+        'server port to connect to.')
+  params = ParseArgs(options)
+  PostTryJob(params)
+
+
+if __name__ == '__main__':
+  sys.exit(Main(sys.argv))