[Freeipa-devel] [PATCH] enhance ipa-listdelegation

[Rob Crittenden]

Rob Crittenden wrote:
> Martin Nagy wrote:
>> Rob Crittenden wrote:
>>> Add options to display a subset of delegations and return 2 if none
>>> are found.
>>>
>>> rob
>>
>> Some thoughts:
>>
>>>  def parse_options():
>>> -    parser = OptionParser()
>>> +    usage = "%prog [options]"
>>> +    parser = OptionParser(usage)
>>
>> No need for usage, "%prog [options]" is the default.
> 
> Ok.
> 
>>
>>> +    parser.add_option("-s", "--source", dest="source",
>>> +                      help="Source group of delegation")
>>> +    parser.add_option("-n", "--name", dest="name",
>>> +                      help="Name of delegation")
>>> +    parser.add_option("-t", "--target", dest="target",
>>> +                      help="Target group of delegation")
>>>      parser.add_option("--usage", action="store_true",
>>>                        help="Program usage")
>>
>> I suggest we also remove the --usage option, since it's the same as
>> --help if you use parser.print_help()
> 
> I thought of that too but for continuity I decided to leave it. With 2.0 
> it'll be gone.
> 
>>
>>>      parser.add_option("-v", "--verbose", action="store_true",
>>> dest="verbose", @@ -56,14 +60,19 @@ def parse_options():
>>>      args = ipa.config.init_config(sys.argv)
>>>      options, args = parser.parse_args(args)
>>>  
>>> +    if options.usage or len(args) != 1:
>>> +        parser.print_help()
>>> +        sys.exit(1)
>>> +
>>
>> This can be accomplished by parser.error("too many arguments") (without
>> the need for sys.exit(1).
> 
> Ok.
> 
>>
>>> +        if (all or options.name == a.name or
>>> +            options.source == group_dn_to_cn[a.source_group] or
>>> +            options.target == group_dn_to_cn[a.dest_group]):
>>
>> Hm, wouldn't it be better if we required that all conditions are met
>> in order to display them?
> 
> No, one may want to look at only one delegation, or to see all 
> delegations that affect group foo or see all delegations that group bar 
> owns. Or they may want to list them all.
> 
>>
>>> +            print "Delegation Name: " + a.name
>>> +            print "Group " + group_dn_to_cn[a.source_group] 
>>
>> Trailing whitespace (it was there before, but still :)
> 
> Yeah, the bane of my existence. I still blame Karl.
> 
> New patch coming soon.
> 

New patch attached.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-79-delegation2.patch
Type: text/x-patch
Size: 3791 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20080822/7fd6b63a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3245 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20080822/7fd6b63a/attachment-0001.bin>