[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