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

[Petr Viktorin]

On 07/23/2013 01:30 PM, Martin Kosek wrote:
> On 07/19/2013 01:10 PM, 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.
>
> ACK. Pushed both Petr's and Honza's patch to master and ipa-3-2.
>
> Martin

These patches sometimes add the attribute "no_members" to groups, which 
results in 7 tests being broken like this:

======================================================================
FAIL: test_cli.TestCLIParsing.test_group_add
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in 
runTest
     self.test(*self.arg)
   File 
"/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/test_cmdline/test_cli.py", 
line 73, in test_group_add
     version=API_VERSION)
   File 
"/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/test_cmdline/test_cli.py", 
line 24, in check_command
     util.assert_deepequal(kw_expected, kw_got)
   File 
"/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/util.py", 
line 338, in assert_deepequal
     doc, sorted(missing), sorted(extra), expected, got, stack
AssertionError: assert_deepequal: dict keys mismatch.

   missing keys = []
   extra keys = ['no_members']
   expected = {'all': False, 'cn': u'tgroup1', 'raw': False, 'version': 
u'2.62', 'external': False, 'nonposix': False, 'description': u'Test group'}
   got = {'all': False, 'description': u'Test group', 'no_members': 
False, 'raw': False, 'version': u'2.62', 'external': False, 'nonposix': 
False, 'cn': u'tgroup1'}
   path = ()

-- 
Petr³