[Freeipa-devel] [WIP] Thin client
Jan Cholasta
jcholast at redhat.com
Wed May 25 14:07:21 UTC 2016
On 25.5.2016 15:03, David Kupka wrote:
> On 18/05/16 08:01, Jan Cholasta wrote:
>> On 16.5.2016 16:35, Martin Basti wrote:
>>>
>>>
>>> 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.
>>
>> It always returned None for unset params.
>>
>>> LGTM
>>>
>>> * ipalib: optimize API.txt
>>> this contains a lot of black magic, but because this is mainly copy of
>>> current to proper places, LGTM
>>
>> It's actually mostly cut-n-paste.
>>
>>>
>>> * makeaci: load additional plugins using API.add_module
>>> Looks good, but I miss explanation why is this change needed
>>
>> The next change would be impossible without this.
>>
>>>
>>> * 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
>>>
>>>
>>
>>
>
> Hello,
> first batch of 30 patches from Honza's trac-4739 branch
> (https://github.com/jcholast/freeipa/commits/trac-4739) is ready to be
> pushed into master.
> All upto "frontend: allow commands to have an argument named `name`" got
> over numerous test&fix cycles in last week+ and is working as expected
> now, ACK.
Thanks for the review.
>
> Honzo, please rebase and push them, thanks!
>
Attaching the patches for reference.
Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055
--
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-558-rpc-do-not-crash-when-unable-to-parse-JSON.patch
Type: text/x-patch
Size: 888 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-559-parameters-remove-unused-ConversionError-and-Validat.patch
Type: text/x-patch
Size: 16976 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-560-rpc-include-structured-error-information-in-response.patch
Type: text/x-patch
Size: 30569 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-561-frontend-re-raise-remote-RequirementError-using-CLI-.patch
Type: text/x-patch
Size: 13209 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-562-frontend-remove-the-unused-Command.soft_validate-met.patch
Type: text/x-patch
Size: 3251 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-563-frontend-perform-argument-value-validation-only-on-s.patch
Type: text/x-patch
Size: 3996 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-564-batch-do-not-crash-when-no-argument-is-specified.patch
Type: text/x-patch
Size: 761 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-565-ipalib-make-optional-positional-command-arguments-ac.patch
Type: text/x-patch
Size: 11978 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-566-frontend-do-not-forward-unspecified-positional-argum.patch
Type: text/x-patch
Size: 1306 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-567-user-do-not-assume-the-preserve-flags-have-value-in-.patch
Type: text/x-patch
Size: 1412 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-568-frontend-do-not-forward-argument-defaults-to-server.patch
Type: text/x-patch
Size: 1991 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-569-makeapi-optimize-API.txt.patch
Type: text/x-patch
Size: 703969 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-570-ipalib-remove-the-unused-csv-argument-of-Param.patch
Type: text/x-patch
Size: 19532 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-571-makeaci-load-additional-plugins-using-API.add_module.patch
Type: text/x-patch
Size: 1068 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0013.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-572-plugable-replace-API.import_plugins-with-new-API.add.patch
Type: text/x-patch
Size: 5047 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0014.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-573-ipalib-ipaserver-migrate-all-plugins-to-Registry-bas.patch
Type: text/x-patch
Size: 21431 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0015.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-574-ipalib-ipaserver-fix-incorrect-API.register-calls-in.patch
Type: text/x-patch
Size: 8638 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0016.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-575-plugable-remove-the-unused-deprecated-API.register-m.patch
Type: text/x-patch
Size: 1559 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0017.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-576-plugable-switch-API-to-Registry-based-plugin-discove.patch
Type: text/x-patch
Size: 8307 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0018.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-577-frontend-merge-baseldap.CallbackRegistry-into-Comman.patch
Type: text/x-patch
Size: 7860 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0019.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-578-frontend-move-the-interactive_prompt-callback-type-t.patch
Type: text/x-patch
Size: 5584 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0020.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-579-automount-do-not-inherit-automountlocation_import-fr.patch
Type: text/x-patch
Size: 1701 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0021.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-580-dns-move-code-called-on-client-to-the-module-level.patch
Type: text/x-patch
Size: 13787 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0022.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-581-dns-do-not-rely-on-server-data-structures-in-code-ca.patch
Type: text/x-patch
Size: 16085 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0023.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-582-otptoken-fix-import-of-DN.patch
Type: text/x-patch
Size: 1416 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0024.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-583-otptoken_yubikey-fix-otptoken_add_yubikey-arguments.patch
Type: text/x-patch
Size: 5750 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0025.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-584-vault-move-client-side-code-to-the-module-level.patch
Type: text/x-patch
Size: 10198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0026.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-585-vault-copy-arguments-of-client-commands-from-server-.patch
Type: text/x-patch
Size: 11398 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0027.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-586-ipalib-use-relative-imports-for-cross-plugin-imports.patch
Type: text/x-patch
Size: 29683 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0028.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-587-frontend-allow-commands-to-have-an-argument-named-na.patch
Type: text/x-patch
Size: 1296 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160525/46aafcdd/attachment-0029.bin>
More information about the Freeipa-devel
mailing list