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

Thu Apr 4 17:26:39 UTC 2013

Martin Kosek wrote:
> 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").

I chatted with Petr about this in IRC and I think this is the same 
conclusion we came to. I don't think we should return an error with 
these mutually-exclusive options, just return nothing.

As Petr pointed out, adding additional flags does an AND operation and 
since there normally isn't an intersection with these flags returning 
nothing is perfectly ok.

rob