Introduce aliases and regex in submit_try

tools/submit_try

 #!/usr/bin/python
 
 # Copyright (c) 2013 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.
 
 
 """
 submit_try: Submit a try request.
 
 This is a thin wrapper around the try request utilities in depot_tools which
 adds some validation and supports both git and svn.
 """
 
 
 import httplib
 import json
 import os
+import re
 import subprocess
 import svn
 import sys
 
 
 GLOBAL_VARIABLES = json.loads(svn.Cat('http://skia.googlecode.com/svn/'
                                       'buildbot/site_config/'
                                       'global_variables.json'))
 
 
 def GetGlobalVariable(var_name):
   return GLOBAL_VARIABLES[var_name]['value']
 
 
 # Alias which can be used to run a try on every builder.
 ALL_BUILDERS = 'all'
+# Alias which can be used to run a try on all compile builders.
+COMPILE_BUILDERS = 'compile'
+# Alias which can be used to run a try on all builders that are run in the CQ.
+CQ_BUILDERS = 'cq'
+# Alias which can be used to specify a regex to choose builders.
+REGEX = 'regex'
+
+ALL_ALIASES = [ALL_BUILDERS, COMPILE_BUILDERS, CQ_BUILDERS, REGEX]
 
 # Contact information for the build master.
 SKIA_BUILD_MASTER_HOST = str(GetGlobalVariable('master_host'))
 SKIA_BUILD_MASTER_PORT = str(GetGlobalVariable('external_port'))
 
 # All try builders have this suffix.
 TRYBOT_SUFFIX = '_Trybot'
 
 # Location of the codereview.settings file in the Skia repo.
 SKIA_URL = 'skia.googlecode.com'
@@ skipping 50 matching lines @@
   connection = httplib.HTTPConnection(SKIA_URL)
   connection.request('GET', CODEREVIEW_SETTINGS)
   content = connection.getresponse().read()
   for line in content.split('\n'):
     if line.startswith(TRYSERVER_SVN_URL):
       return line[len(TRYSERVER_SVN_URL):].rstrip()
   raise Exception('Couldn\'t determine the TRYSERVER_SVN_URL. Make sure it is '
                   'defined in the %s file.' % CODEREVIEW_SETTINGS)
 
 
-def RetrieveTrybotList():
+def RetrieveTrybotList(json_filename):
   """ Retrieve the list of known trybots from the build master, stripping
   TRYBOT_SUFFIX from the name. """
   trybots = []
   connection = httplib.HTTPConnection(SKIA_BUILD_MASTER_HOST,  
-                                      SKIA_BUILD_MASTER_PORT)  
-  connection.request('GET', '/json/builders')  
+                                      SKIA_BUILD_MASTER_PORT)
+  connection.request('GET', '/json/%s' % json_filename) 
   response = connection.getresponse()  
   builders = json.load(response)
 
   for builder in builders:
     if builder.endswith(TRYBOT_SUFFIX):
       trybots.append(builder[:-len(TRYBOT_SUFFIX)])
   return trybots
 
 
 def ValidateArgs(argv, trybots, is_svn=True):
@@ skipping 19 matching lines @@
       return self._changelist
 
     @property
     def revision(self):
       return self._revision
 
   usage = (
 """submit_try: Submit a try request.
 submit_try %s--bot <buildername> [<buildername> ...]
 
--b, --bot           Builder on which to run the try. Required.
+-b, --bot           Builder(s) or Alias on which to run the try. Required.
+                    Allowed aliases: %s
 -h, --help          Show this message.
 -r <revision#>      Revision from which to run the try.
--l, --list_bots     List the available try builders and exit.
-""" % ('<changelist> ' if is_svn else ''))
+-l, --list_bots     List the available try builders and aliases and exit.
+""" % ('<changelist> ' if is_svn else '', ALL_ALIASES))
 
   def Error(msg=None):
     if msg:
       print msg
     print usage
     sys.exit(1)
 
   using_bots = None
   changelist = None
   revision = None
 
   while argv:
     arg = argv.pop(0)
     if arg == '-h' or arg == '--help':
       Error()
     elif arg == '-l' or arg == '--list_bots':
-      print 'submit_try: Available builders:\n  %s' % '\n  '.join(trybots)
+      format_args = ['\n  '.join(trybots)] + ALL_ALIASES

[borenet, 2013/04/08 21:02:21]

I might put the aliases first, since the list of trybots will be long.

[rmistry, 2013/04/08 21:11:38]

They have to be in the same order as required by the string below or did you
mean something else?

[borenet, 2013/04/08 21:15:50]

I wrote the comment before I really understood the code. Please ignore :)

+      print (
+"""
+submit_try: Available builders:\n  %s
+
+Can also use the following aliases to run on groups of builders-
+  %s: Will run against all trybots.
+  %s: Will run against all compile trybots.
+  %s: Will run against the same trybots as the commit queue.
+  %s: You will be prompted to enter a regex to select builders with.
+
+""" % tuple(format_args))

[borenet, 2013/04/08 21:02:21]

This seems like a strange way to do this, since the string to print makes the
assumption that format_args will be: (somelistoftrybots, "all", "compile", "cq",
"regex")

Why not just:

"....." % ('\n  '.join(trybots), ALL_BUILDERS, COMPILE_BUILDERS, CQ_BUILDERS,
REGEX)

?

[rmistry, 2013/04/08 21:11:38]

This is how I did it first but then I worried about people adding more aliases
and not updating documentation. This way if an alias is added and not documented
here then it will fail.
Though I dont have a strong preference either way.

[borenet, 2013/04/08 21:15:50]

That makes sense. SGTM

       sys.exit(0)
     elif arg == '-b' or arg == '--bot':
       if using_bots:
         Error('--bot specified multiple times.')
       if len(argv) < 1:
         Error('You must specify a builder with "--bot".')
       using_bots = []
       while argv and not argv[0].startswith('-'):
         for bot in argv.pop(0).split(','):
-          if bot == ALL_BUILDERS:
+          if bot in ALL_ALIASES:
             if using_bots:
-              Error('Cannot specify "all" with additional builder names.')
-            using_bots = trybots
+              Error('Cannot specify "%s" with additional builder names or '
+                    'aliases.' % bot)
+            if bot == ALL_BUILDERS:
+              using_bots = trybots
+            elif bot == COMPILE_BUILDERS:
+              using_bots = filter(lambda trybot: '_Compile_' in trybot, trybots)

[borenet, 2013/04/08 21:02:21]

Is this faster than a list comprehension?

[rmistry, 2013/04/08 21:11:38]

List comprehension is marginally faster but the performance difference is
negligible in this (and majority of other) cases. People who prefer one of the
two are divided in different camps in python, it all comes down to personal
preference.

[borenet, 2013/04/08 21:15:50]

I guess I find the list comprehension more readable here, but I'll leave it up
to you.

[rmistry, 2013/04/09 11:33:39]

Changed to list comprehension. I did some more research yesterday and seems that
Python's BDFL wanted to remove filter but did not because of user feedback,
maybe that makes list comprehension more pythonic.

+            elif bot == CQ_BUILDERS:
+              using_bots = RetrieveTrybotList(json_filename='cqtrybots')
+            elif bot == REGEX:
+              while True:
+                regex = raw_input("Enter your trybot regex: ")
+                p = re.compile(regex)
+                using_bots = filter(lambda trybot: p.match(trybot), trybots)
+                print '\n\nTrybots that match your regex:\n%s\n\n' % '\n'.join(using_bots)

[borenet, 2013/04/08 21:02:21]

Nit: 80 chars.

[rmistry, 2013/04/08 21:11:38]

Done.

+                if raw_input('Re-enter regex? [y,n]: ') == 'n':
+                  break
             break
           else:
             if not bot in trybots:
               Error('Unrecognized builder: %s' % bot)
             using_bots.append(bot)
     elif arg == '-r':
       if len(argv) < 1:
         Error('You must specify a revision with "-r".')
       revision = argv.pop(0)
     else:
@@ skipping 45 matching lines @@
                 '--svn_repo', GetTryRepo(),
                 '--root', GetCheckoutRoot(is_svn),
                 '--bot', botlist]
     if args.revision:
       try_args.extend(['-r', args.revision])
     trychange.TryChange(try_args, None, False)
 
 
 def main():
   # Retrieve the list of active try builders from the build master.
-  trybots = RetrieveTrybotList()
+  trybots = RetrieveTrybotList(json_filename='builders')

[borenet, 2013/04/08 21:02:21]

Actually, now that we have the /json/trybots page, we can make this 'trybots'
right?

[rmistry, 2013/04/08 21:11:38]

Yes right. Done.

 
   # Determine if we're in an SVN checkout.
   is_svn = os.path.isdir('.svn')
 
   # Parse and validate the command-line arguments.
   args = ValidateArgs(sys.argv[1:], trybots=trybots, is_svn=is_svn)
 
   # Submit the try request.
   SubmitTryRequest(args, is_svn=is_svn)
 
 
 if __name__ == '__main__':
   sys.exit(main())