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

Tue Apr 9 14:38:14 UTC 2013

On 04/04/2013 12:02 PM, 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").
>
> Martin
>
New version attached.

-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0267-1-Filter-groups-by-type-POSIX-non-POSIX-external.patch
Type: text/x-patch
Size: 11759 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130409/8353fd32/attachment.bin>