[Freeipa-devel] [PATCH] 267 Filter groups by type (normal, posix, external)

[Martin Kosek]

On 04/04/2013 11:48 AM, Tomas Babej wrote:
> On 03/22/2013 03:03 PM, Martin Kosek wrote:
>> On 03/21/2013 06:10 PM, Petr Vobornik wrote:
>>> On 03/21/2013 05:10 PM, Martin Kosek wrote:
>>>> On 03/16/2013 03:32 AM, Endi Sukma Dewata wrote:
>>>>> On 3/12/2013 11:28 AM, Petr Vobornik wrote:
>>>>>> Here's a patch for filtering groups by type.
>>>>>> Design page: http://www.freeipa.org/page/V3/Filtering_groups_by_type
>>>>>>
>>>>>> The interface is:
>>>>>>> StrEnum('type?',
>>>>>>>       cli_name='type',
>>>>>>>       label=_('Type'),
>>>>>>>       doc=_('Group type'),
>>>>>>>       values=(u'posix', u'normal', u'external'),
>>>>>>> ),
>>>>>> I have two design questions.
>>>>>> 1. Is --type the right option name?
>>>>> Fine by me, it matches the label and description.
>>>>>
>>>>>> 2. Is `normal` the right name for non-posix, non-external group? The
>>>>>> default group type (when adding group) is posix. Should the name be
>>>>>> something else: `simple`, `plain`, `ordinary`?
>>>>> We also use 'normal' in the group adder dialog, so it's consistent. Other
>>>>> options are 'basic', 'standard', 'regular'.
>>>>>
>>>>>> I didn't want to create an option for each type. IMO it brings more
>>>>>> complexity.
>>>>> Maybe the group-add/mod command should use the same --type option?
>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/3483
>>>>> ACK from me, but maybe others might have some comments.
>>>>>
>>>> I am just thinking about if the new API is right. For example, when we add an
>>>> external group, we use ipa group-add --external. But when we search for
>>>> external groups, we suddenly use
>>>> # ipa group-find --type=external
>>>> and not
>>>> # ipa group-find --external
>>>> or
>>>> # ipa group-find --nonposix
>>>>
>>>> Wouldn't that cause confusion? I am looking for same second opinion on this
>>>> one.
>>>>
>>>> I also did not like "normal" group type very much, maybe we should just
>>>> call it
>>>> "nonposix"? As that's the option you use when you are creating such group:
>>>> # ipa group-add --nonposix foo
>>>>
>>>> Otherwise, the patch looks good functionally.
>>>>
>>>> Martin
>>>>
>>> I have to note that external group is also non-posix. Following command is
>>> valid:
>>>    # ipa group-add foo --desc=a --external --nonposix
>>>
>>> By that logic
>>>    # ipa group-find --nonposix
>>>
>>> Would also list external groups.
>>>
>>> I fine with renaming 'normal' to something better (will also require Web UI
>>> change), but it is not 'nonposix'.
>> I think this logic is flawed as well. Then you could say that posix group is
>> also nonposix, because it contains the same objectclasses as nonpoxis group +
>> posixGroup objectclass.
>>
>> "nonposix" is the term we already use (see --nonposix), not something
>> artificial or new, so I would not be afraid of it.
>>
>> Martin
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> Let us try to move on with this, here are my 2 cents:
> 
> 1.) normal is not a suitable name for non-posix, non-external group. As a user,
> I would assume that
>   # ipa group-find --type=normal
> 
> would return the groups that I created using simple
>   # ipa group-add testgroup
> 
> command. By that logic, any other suggested synonym implying there's nothing
> special about this
> group is not suitable.
> 
> 2.) If not normal (or any other synonym implying there's nothing special about
> this group) then what?
> We can either:
>   - use exact but complicated --non-posix-non-external
>   - use --nonposix and deal with the fact that sets defined by the type are not
> disjunct
>   - make up our own new term and define it
> 
> While none of these options are fortunate, let's look for the least resistance:
>   - exact, but complicated names are ugly and do not keep interface simple
>   - nonposix groups are superset of external groups
>   - confuses the user and makes the learning curve steeper
> 
> From this I would go for option 2, indeed, if you think about --nonposix /
> --external as flags, where
> the external takes priority before nonposix, this kind of makes sense. If the
> user does not think
> about the implementation (that every external group is nonposix), he may indeed
> find himself in this mindset.
> 
> 3.) I'm fine both with --type=external and --external approaches. The latterr
> is more consistent with the way we do things,
> *-find commands search mainly on selected subset of attributes, so using the
> flag analogy I mentioned an paragraph ago,
> you would expect --external to behave as an attribute, especially if group-add
> command accepts it in this form.
> 
> Having 3 options instead of one will clutter things a bit more, but if we keep
> them in the same place (in the list of options)
> it should not cause much confusion, more so if the descriptions would be nearly
> the same, one would quickly see that these
> belong together.
> 
> Tomas
> 

Thanks Tomas for your opinion, I can agree with that. To make it more in an
actual design, this is API following this discussion that I would propose:

This is API we already have in IPA:
ipa group-add --external
ipa group-add --nonposix
ipa group-find --private

This is API that I would propose to add to be consistent with what we already have:
ipa group-find --nonposix
ipa group-find --posix
ipa group-find --external

--nonposix would only match groups added with --nonposix flag in group-add,
i.e. no --external groups.

As Tomas said, these should also be close together. We can even add a specific
option group for them, like there are with ipa dnsrecord-add, named for example
"Group Types". We may also raise OptionError when these option are used
together to make this less confusing - e.g. OptionError("group type options
(--nonposix, --posix and --external) are mutually exclusive").

Martin