[Freeipa-devel] [PATCHES 0146-0152] Server Roles v2

[Jan Cholasta]

On 9.6.2016 17:06, Martin Babinsky wrote:
> On 06/09/2016 03:54 PM, Petr Vobornik wrote:
>> On 06/09/2016 01:02 PM, Martin Babinsky wrote:
>>> On 06/07/2016 07:01 PM, Pavel Vomacka wrote:
>>>>
>>>>
>>>> On 06/07/2016 12:07 PM, Martin Babinsky wrote:
>>>>> On 06/03/2016 05:25 PM, Martin Babinsky wrote:
>>>>>> I am sending rebased patches implementing
>>>>>> http://www.freeipa.org/page/V4/Server_Roles
>>>>>>
>>>>>> I hope the patches work since I have had a lot of fun rebasing
>>>>>> them on
>>>>>> top of thin client and DNS locations effort.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/5181
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Sending updated patches according to Jan's interactive review.
>>>>>
>>>>> Since the name of attributes returned by API commands and signature of
>>>>> `server-role-find` have changed, a small update in WebUI patches is
>>>>> required.
>>>>>
>>>>>
>>>>>
>>>> NACK, why did you remove sizelimit from server_role_find command's? Is
>>>> it possible to return it back? It breaks WebUI.
>>>
>>> Indeed, this was caused by changing the base class of the command. It is
>>> fixed in updated patches.
>>>
>>
>> NACK
>>
>> Option timelimit? of command server_role_find in ipalib, not in API file:
>> Int('timelimit?', autofill=False)
>> Option sizelimit? of command server_role_find in ipalib, not in API file:
>> Int('sizelimit?', autofill=False)
>>
>> There are one or more changes to the API.
>> Either undo the API changes or update API.txt and increment the major
>> version in VERSION.
>> Makefile:159: recipe for target 'version-update' failed
>> make: *** [version-update] Error 1
>>
>
> Oops, seems like a missed API.txt update.
>
> Fixed.

"ipa server-role-find" does not return the "IPA master" role for my 
server ("ipa-server-role $HOSTNAME 'IPA master'" does).

I would rather skip the option altogether rather than hide it:

+            # we do not want to test negative membership for roles
+            # hide it from CLI
+            elif option.name == 'no_servrole':
+                option = option.clone(flags={'no_option'})

The patches need a rebase (VERSION).

Otherwise LGTM.

-- 
Jan Cholasta