[Freeipa-devel] [WIP] Thin client

[Martin Basti]

On 09.05.2016 14:07, Jan Cholasta wrote:
> On 6.5.2016 14:32, Martin Basti wrote:
>>
>>
>> On 28.04.2016 14:45, Jan Cholasta wrote:
>>> Hi,
>>>
>>> I have pushed my thin client WIP branch to GitHub:
>>> <https://github.com/jcholast/freeipa/tree/trac-4739>.
>>>
>>> All commits up to "ipalib: use relative imports for cross-plugin
>>> imports" should be good for review. The rest is subject to change
>>> (WARNING: I will force push into this branch).
>>>
>>> Honza
>>>
>> I did not test it yet, I just checked the code
>>
>> * automount: do not inherit automountlocation_tofiles from LDAPQuery *
>> LGTM
>>
>> * dns: move all dnsrecord code called on client to a single class *
>> LGTM
>>
>> * dns: do not rely on server data structures in code called on client *
>> 1)
>> you forgot to increment VERSION
>
> This was deliberate, as it will no longer be necessary to bump VERSION 
> for backward compatible changes after this whole patchset is merged. 
> But we're not there yet, so fixed.
>
How we should handle VERSION after your patches?
>>
>> Otherwise LGTM
>>
>> * otptoken: fix import of DN *
>> ACK
>>
>> * otptoken_yubikey: fix otptoken_add_yubikey arguments *
>> 1)
>> you forgot to increment VERSION
>
> Fixed.
>
>>
>> 2)
>> Did you find out why this was issue?
>> -        del answer['value']    # Why does this cause an error if 
>> omitted?
>>  -        del answer['summary']  # Why does this cause an error if 
>> omitted?
>
> The command definition was not complete, it was missing has_output.
>
>>
>> Otherwise LGTM
>>
>> * vault: move client-side code to the module level *
>> LGTM
>>
>> * vault: copy arguments of client commands from server counterparts *
>> 1)
>> you forgot to increment Version
>
> Fixed.
>
>>
>> * ipalib: use relative imports for cross-plugin imports *
>> 1) Missing explanation for future generations why this change is needed
>> in commit message
>
> Fixed.
>
>>
>> The other commits I will check later.
>> Martin^2
>>
>
> Okay. Thanks.
>

* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent 
found anything better to use.
LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of 
current to proper places, LGTM

* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
  LGTM

Martin^2