[PATCH 6/n] pwclient: diagnose hash_parser errors gracefully

[Bernhard Reutner-Fischer]

[Please keep me CC'ed]

On Fri, Aug 29, 2014 at 03:14:35PM +0000, Keller, Jacob E wrote:
>On Fri, 2014-08-29 at 14:02 +0200, Bernhard Reutner-Fischer wrote:

>> @@ -618,6 +620,23 @@ def main():
>>      if hash_str != None:
>>          patch_ids = [patch_id_from_hash(rpc, project_str, hash_str)]
>>  
>> +    # helper for non_empty() to print correct helptext
>> +    h = None
>> +    try:
>> +        h = locals()[action + '_parser']
>> +    except:
>> +        pass # never happens
>
>This is bad. Why blindly pass any exceptions here?? If you "know" it
>never happens then you shouldn't have an except block, since that will
>prevent debugging real exceptions that could occur. Unless you are
>specifically blocking some exception you know will happen? but then it
>should be typed to only catch the ones you know of.......

Iff we update to an argparse that supports aliases and we do not
deprecate action == "search" as an alias for "list" then we might end up
with no search_parser here. As that is not a real show-stopper, i did
choose to just not print the corresponding parser's help-text if there
is no such parser. Note that this currently cannot happen since we
cannot use aliases with argparse from python-2.7 (AFAICS).

thanks,
>
>> +    # Require either hash_str or IDs for
>> +    def non_empty(h, patch_ids):
>> +        """Error out if no patch IDs were specified"""
>> +        if patch_ids == None or len(patch_ids) < 1:
>> +            sys.stderr.write("Error: Missing Argument! " +
>> +              "Either [-h HASH] or [ID [ID ...]] are required\n")
>> +            if h:
>> +                h.print_help()
>> +            sys.exit(1)
>> +        return patch_ids
>> +
>>      if action == 'list' or action == 'search':
>>          if args.get('patch_name') != None:
>>              filt.add("name__icontains", args.get('patch_name'))
>> @@ -630,29 +649,31 @@ def main():
>>          action_states(rpc)
>>  
>>      elif action == 'view':
>> -        for patch_id in patch_ids:
>> +        for patch_id in non_empty(h, patch_ids):
>>              s = rpc.patch_get_mbox(patch_id)
>>              if len(s) > 0:
>>                  print unicode(s).encode("utf-8")