[Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups

[Petr Vobornik]

On 07/19/2013 03:49 PM, Jan Cholasta wrote:
> On 19.7.2013 13:10, Petr Vobornik wrote:
>> On 07/18/2013 05:29 PM, Jan Cholasta wrote:
>>> On 18.7.2013 17:26, Martin Kosek wrote:
>>>> On 07/18/2013 05:22 PM, Jan Cholasta wrote:
>>>>> On 18.7.2013 17:07, Martin Kosek wrote:
>>>>>> On 07/18/2013 04:53 PM, Jan Cholasta wrote:
>>>>>>> Added patch which adds new hidden option no_members to suppress
>>>>>>> membership
>>>>>>> processing for commands of all objects that have member attributes.
>>>>>>> This can be
>>>>>>> used by the WebUI to prevent member lookups where they are not
>>>>>>> necessary (as we
>>>>>>> discussed off-line with Martin and Petr).
>>>>>>>
>>>>>>> Honza
>>>>>>>
>>>>>>
>>>>>> 1) Should the new option really have "exclude='webui'? I thought
>>>>>> that Web UI is
>>>>>> the main and only consumer of this option.
>>>>>
>>>>> The 'webui' context doesn't actually exist and the only meaning of
>>>>> this stanza
>>>>> is that the option is not shown in the output of the show_mappings
>>>>> command.
>>>>>
>>>>>>
>>>>>> 2) I would clearly state this is an internal option only, e.g.
>>>>>>
>>>>>> + doc=_('INTERNAL: suppress processing of membership attributes.'),
>>>>>
>>>>> No other internal option has this kind of thing in its doc and nobody
>>>>> will see
>>>>> it anyway, so we might just leave it like that IMHO.
>>>>
>>>> OK.
>>>>
>>>>>
>>>>>>
>>>>>> 3) It would be nice to state that this option is mutually exclusive
>>>>>> with --all,
>>>>>> but given it is internal anyway and there is no central place to
>>>>>> define it, we
>>>>>> do not need to do that.
>>>>>
>>>>> The options are not really mutually exclusive at this point, they
>>>>> can be
>>>>> specified together, --all takes precedence.
>>>>
>>>> Well, they can be specified together, but the option is then NOOP
>>>> which could
>>>> confuse users which may have different expectations. Being explicit
>>>> helps.
>>>
>>> I agree.
>>>
>>>> But
>>>> as I said, in this case this is not a requirement.
>>>
>>> I agree as well :-)
>>>
>>> Honza
>>>
>>
>> Functional ACK for Honza's patch (didn't break Web UI test suite)
>>
>> Attaching Web UI patch.
>>
>> It:
>> 1) Removed --all from _find and _show commands used by search pages. All
>> displayed attributes should be already included in default attributes.
>>
>> 2) Removed search_all_attributes - Not needed since introduction of
>> paging.
>>
>> 3) Added --no-members options to search _show commmands.
>
> Hmm, is the has_members function used to distinguish between objects
> with members and without members? If so, would it be helpful to add the
> no_members option to all commands (just like "all" and "raw"), so that
> you don't have to do this check?
>
> Honza
>

Yes, that's its purpose. IMO it doesn't matter now, the function is 
implemented. The only benefits would be small, not noticeable, 
performance gain and tolerance to incorrect use of --no-members. Doesn't 
look like a reason for polluting all commands with it.
-- 
Petr Vobornik