[Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

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.

>
> 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.

-- 
Jan Cholasta