Introduce aliases and regex in submit_try

tools/submit_try

Index: tools/submit_try
===================================================================
--- tools/submit_try	(revision 8554)
+++ tools/submit_try	(working copy)
@@ -16,6 +16,7 @@
 import httplib
 import json
 import os
+import re
 import subprocess
 import svn
 import sys
@@ -32,7 +33,15 @@
 
 # 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'))
@@ -102,13 +111,13 @@
                   '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)
 
@@ -148,11 +157,12 @@
 """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:
@@ -169,7 +179,18 @@
     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:
@@ -179,10 +200,24 @@
       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:
@@ -248,7 +283,7 @@
 
 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')