On 07/23/2013 04:54 PM, Petr Viktorin wrote:
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 = ()


Correction: they don't add the attribute to output. It's just that the CLI tests need to be updated with the new option.

--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to