[Freeipa-devel] [PATCH] 461 ignore no_* options in Virtual class

[Rob Crittenden]

Pavel Zuna wrote:
> On 06/02/2010 08:35 PM, Rob Crittenden wrote:
>> The Virtual base class is used for doing non-LDAP operations, right not
>> just for certificate commands. It wasn't honoring the no_* option flags.
>> Add support for that.
>>
>> rob
>>
> NACK.
> 
> I think I do understand what this patch is trying to do and it works, 
> but I also think it builds over the misuse of the 'no_*' option flags.
> 
> These flags were added, so that we can specify a common list of Params 
> in Object.takes_params and mark those that are inappropriate for 
> Add/Update/Search methods.
> 
> It doesn't make sense to use these flags in cert.py commands, because 
> they aren't linked to any Object.
> 
> If you want to add Params just for output purpose, use 
> Command.has_output_params.
> 
> EXAMPLE: WRONG:
> 
> class cert_status(VirtualCommand):
>     takes_args = (
>         Str('request_id',
>             label=_('Request id'),
> # pzuna: the next line has no effect
>             flags=['no_create', 'no_update', 'no_search'],
>         ),
>     )
> # pzuna: ipa cert-status doesn't TAKE the cert-request-status option
>     takes_options = (
>         Str('cert_request_status?',
>             label=_('Request status'),
>             flags=['no_create', 'no_update', 'no_search'],
>         ),
>     )
> 
> 
> EXAMPLE: RIGHT:
> 
> class cert_status(VirtualCommand):
>     takes_args = (
>         Str('request_id',
>             label=_('Request id'),
>         ),
>     )
>     has_output_params = (
>         Str('cert_request_status?',
>             label=_('Request status'),
>         ),
>     )
> 
> 
> Pavel

Okay, I see you're point. We can fix the cert plugin with 
has_output_params, but it still is an incompatibility with other classes.

rob