Add some debugging to buildbot_globals, sort the trybot list

tools/buildbot_globals.py

 #!/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.
 
 """
 Provides read access to buildbot's global_variables.json .
 """
 
 import json
 import svn
 
 _global_vars = None
 
+
+GLOBAL_VARS_JSON_URL = ('http://skia.googlecode.com/svn/buildbot/site_config/'

[epoger, 2013/12/02 18:36:28]

Personally, I would write it this way so the URL would not be split (thus giving
better results if someone is grepping thru the code):

GLOBAL_VARS_JSON_URL = (
    'http://skia.googlecode.com/svn/buildbot/site_config/global_variables.json')

But either way is fine.

Damn you, Python.

[borenet, 2013/12/02 20:22:24]

Done.

+                        'global_variables.json')
+
+
+class GlobalVarsRetrievalError(Exception):

[epoger, 2013/12/02 18:36:28]

Please add docstrings giving a brief description of these new Exceptions (what
it means if one is raised)

[borenet, 2013/12/02 20:22:24]

Done.

+  pass
+
+
+class JsonDecodeError(Exception):

[rmistry, 2013/12/02 14:57:12]

simplejson has a JSONDecodeError. Also, I wonder if we should be using
simplejson everywhere instead of json.

[borenet, 2013/12/02 16:26:29]

Probably a good decision in the long run.  All I care about with this change is
getting better error messages; if simplejson can provide that, great!  For now,
I'd like to keep using the json module, since some bots (I tested the GCE bots)
don't have simplejson installed, and I'm hesitant to add another dependency
unless we really need it.

[rmistry, 2013/12/02 16:28:24]

Makes sense.

+  pass
+
+
 class NoSuchGlobalVariable(KeyError):
   pass
 
+
 def Get(var_name):
   '''Return the value associated with this name in global_variables.json.
   Raises NoSuchGlobalVariable if there is no variable with that name.'''
   global _global_vars
   if not _global_vars:
-    _global_vars = json.loads(svn.Cat('http://skia.googlecode.com/svn/'
-                                      'buildbot/site_config/'
-                                      'global_variables.json'))
+    try:
+      global_vars_text = svn.Cat(GLOBAL_VARS_JSON_URL)
+    except Exception:

[epoger, 2013/12/02 18:36:28]

TL;DR : this is fine. :-)

I wondered what the pros and cons of "except Exception:" (vs. just "except:")
were, which led me to http://docs.python.org/2/howto/doanddont.html#except

So it's better to specify Exception, like you did.

Unfortunately, it looks like there is no really great way of wrapping exceptions
in Python 2.x.  There's an interesting writeup at
http://www.ianbicking.org/blog/2007/09/re-raising-exceptions.html

[borenet, 2013/12/02 20:22:24]

Yeah, I guess this isn't ideal, but losing the stack trace here isn't a big
deal, since there's not much code in the try blocks.

+      raise GlobalVarsRetrievalError('Failed to retrieve %s.' %
+                                     GLOBAL_VARS_JSON_URL)
+    try:
+      _global_vars = json.loads(global_vars_text)
+    except ValueError as e:
+      raise JsonDecodeError(e.message + '\n' + global_vars_text)
   try:
     return _global_vars[var_name]['value']
   except KeyError:
     raise NoSuchGlobalVariable(var_name)