[Freeipa-devel] [PATCHES] 0104-0106 Provide means of displaying warning and informational messages on clients
Petr Viktorin
pviktori at redhat.com
Thu Dec 13 17:09:47 UTC 2012
On 12/13/2012 04:43 PM, Martin Kosek wrote:
> On 12/13/2012 10:59 AM, Petr Viktorin wrote:
>> It's time to give this to another set of eyes :)
>>
>> Design document: http://freeipa.org/page/V3/Messages
>> Ticket: https://fedorahosted.org/freeipa/ticket/2732
>>
>> More info is in commit messages.
>>
>>
>> Because of https://fedorahosted.org/freeipa/ticket/3294, I needed to change the
>> design document: when the client doesn't send the API version, it is assumed
>> it's at a version before capabilities were introduced (i.e. 2.47).
>> The client still gets a warning if the version is missing. Except for those
>> commands where IPA didn't send a version -- ping, cert-show, etc. -- the
>> warning wouldn't pass validation on old clients. (I'm assuming that our client
>> is so far the only one that validates so strictly.)
>
> I did a basic test of this patch and also quickly read through the patches and
> besides nitpicks (like unused inspect module in
> tests/test_ipalib/test_messages.py in patch 0105) I did not find any obvious
> errors in the Python code.
Noted, will fix in future versions of the patch.
>
> However, this patch breaks WebUI badly, I did not even get to a log in screen.
> Cooperation with Petr Vobornik will be needed. In my case, I got blank screen
> and Javascript error:
>
> TypeError: IPA.messages.dialogs is undefined
> https://vm-037.idm.lab.bos.redhat.com/ipa/ui/ipa.js
> Line 1460
>
> I assume this is related to the Internal Error that was returned in the JSON call
>
> {
> "error": null,
> "id": null,
> "principal": "admin at IDM.LAB.BOS.REDHAT.COM",
> "result": {
> "count": 5,
> "results": [
> {
> "error": "an internal error has occurred",
> "error_code": 903,
> "error_name": "InternalError"
> },
> {
> ...
>
> This can be reproduced with:
>
> # curl -v -H "Content-Type:application/json" -H
> "referer:https://`hostname`/ipa" -H "Accept:applicaton/json" --negotiate -u :
> --cacert /etc/ipa/ca.crt -d
> '{"method":"i18n_messages","params":[[],{}],"id":0}' -X POST
> https://`hostname`/ipa/json
Good catch! The i18n_messages plugin already defines a "messages"
output. When I renamed this from "warnings" to "messages" I forgot to
check for clashes.
Since i18n_messages is an internal command only used by the Web UI, we
can rename its output to "texts" without breaking compatibility.
I'm attaching a preliminary fix (for both backend and UI), but hopefully
it won't be necessary, see below.
> I am also not sure I like the requirement of a specific version option to be
> always passed. I would prefer that missing version option would mean "I use the
> most recent version of API" instead - it would make the custom JSONRPC/XMLRPC
> calls easier to use.
>
> But since the version option was not being sent for some commands, we may not
> have a choice anyway if we do not want to break old clients in case we add some
> capabilities to these commands.
>
I see three other options, all worse:
- Do not use capabilities for the affected commands, meaning no new
functionality can be added to them (and by extension, no new
functionality common to all commands can be added).
- Treat a missing version as the current version
- Break backwards compatibility
And one possibly better (thanks to Petr¹ and Martin for opening my eyes
off-list!):
- Deprecate XML-RPC. All XML-RPC requests would be pinned to current
version (2.47). Capabilities/messages would only apply to JSON-RPC.
This would also allow us to solve the above name-clashing problem
elegantly. Here is a reminder of what a JSON response looks like:
{
"error": null,
"id": 0,
"principal": "admin at IDM.LAB.BOS.REDHAT.COM",
"result": {
"summary": "IPA server version 3.1.0GIT2e4bd02. API version 2.47"
},
"version": "3.1.0GIT2e4bd02"
}
A XML-RPC response only contains the "result" part of that.
So with JSON, we can put the messages in the top level, which is much
better design.
XML-RPC sucks in other ways too. We already have a workaround for its
inability to attach extra info to errors (commit
88262a75ffe7a25640333dcc4da2100830cae821, Add instructions support to
PublicError).
I've opened a RFC here: https://fedorahosted.org/freeipa/ticket/3299.
--
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0115-Rename-the-messages-Output-of-the-i18n_messages-comm.patch
Type: text/x-patch
Size: 3438 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121213/36b5de1d/attachment.bin>
More information about the Freeipa-devel
mailing list