Improved user-facing messages

pending_manager.py

Index: pending_manager.py
diff --git a/pending_manager.py b/pending_manager.py
index 7548be2e2490eae993d80d98a51ed6c49a320d50..3efb64880af5eabc07217be20ebf7d3756256e22 100644
--- a/pending_manager.py
+++ b/pending_manager.py
@@ -148,14 +148,18 @@ class PendingManager(object):
   # Delay (secs) between commit bursts.
   COMMIT_BURST_DELAY = 10*60
 
-  def __init__(self, context_obj, pre_patch_verifiers, verifiers):
+  def __init__(self, context_obj, pre_patch_verifiers, verifiers,
+               project_name=''):
     """
     Args:
       pre_patch_verifiers: Verifiers objects that are run before applying the
                            patch.
       verifiers: Verifiers object run after applying the patch.
     """
-    assert len(pre_patch_verifiers) or len(verifiers)
+    if not(len(pre_patch_verifiers) or len(verifiers)):
+      raise ValueError('at least one verifier should be defined (in project %s)'
+                       % project_name)

[Paweł Hajdan Jr., 2014/01/06 11:23:26]

You may also want to throw an exception if project_name is empty.

project_name has a default value, and people will skip things that are not
strictly required to make something work.

[pgervais, 2014/01/06 22:22:58]

I added 'project_name' as a convenience, and used it only in one place (the
future bling CQ). It's optional just to be backward-compatible, I didn't want to
risk breaking every commit queue by modifying all of them at once. However, if
you feel that it's worth it, I can do it.

[ghost stip (do not use), 2014/01/09 00:46:28]

You have to specify the project when you start up commit-queue, so I'm not sure
what good having the project name in the error message is if it'll be blank most
of the time.

+
     self.context = context_obj
     self.pre_patch_verifiers = pre_patch_verifiers or []
     self.verifiers = verifiers or []
@@ -429,6 +433,7 @@ class PendingManager(object):
             'Failed to set the flag to False for %s with message %s' % (
               pending.pending_name(), message))
         traceback.print_stack()
+        logging.error(str(e))

[Paweł Hajdan Jr., 2014/01/06 11:23:26]

Could you explain more why? Please note logging.error call above.

[pgervais, 2014/01/06 22:22:58]

str(e) gives the full HTTP status, which is not visible in the stack trace.

         errors.send_stack(e)
       if message:
         try: