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": "ad...@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": "ad...@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³
From 0b94fd2f72e3d771a3a70c39d8cba39f2391994b Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Thu, 13 Dec 2012 11:42:06 -0500
Subject: [PATCH] Rename the "messages" Output of the i18n_messages command to
"texts"
This is to prevent a fatal name clash wih the new common "messages" Output.
Since i18n_messages is an internal plugin, the change does not affect
our public API.
---
API.txt | 2 +-
VERSION | 2 +-
install/ui/ipa.js | 2 +-
install/ui/test/data/ipa_init.json | 2 +-
ipalib/plugins/internal.py | 4 ++--
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/API.txt b/API.txt
index 0c619efd93d5df2ca869bf5de68bf2a4c81b1da7..46a1610847b32409adc16067041a7ff186d84272 100644
--- a/API.txt
+++ b/API.txt
@@ -1921,7 +1921,7 @@ output: Output('value', <type 'unicode'>, None)
command: i18n_messages
args: 0,1,1
option: Str('version?', exclude='webui')
-output: Output('messages', <type 'dict'>, None)
+output: Output('texts', <type 'dict'>, None)
command: idrange_add
args: 1,11,3
arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
diff --git a/VERSION b/VERSION
index 37af5ef73b74500e0cd7397fb2c109332c049bc6..7bcfe6f9628b8193f44faa9d399e3295e3204c1f 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=20100614120000
# #
########################################################
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=48
+IPA_API_VERSION_MINOR=49
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index a33fbfd5e4e066321155f1430c449d5b997551ff..0f69521605a2cd79311fab78d02d58f38d85f56a 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -113,7 +113,7 @@ var IPA = function() {
batch.add_command(IPA.command({
method: 'i18n_messages',
on_success: function(data, text_status, xhr) {
- that.messages = data.messages;
+ that.messages = data.texts;
}
}));
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 44484a9aaa4b04c2730cacd1adcbe682e507d736..c16bc992e437e6a5e1be918d46ea5dda33b97562 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -6,7 +6,7 @@
"results": [
{
"error": null,
- "messages": {
+ "texts": {
"ajax": {
"401": {
"message": "Your session has expired. Please re-login."
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index e9fc9de401e4d77a072fdb2e3671a629a352df6d..692d1d735b541882f6cec3ce9236e2c9e2306849 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -680,10 +680,10 @@ class i18n_messages(Command):
},
}
has_output = (
- Output('messages', dict, doc=_('Dict of I18N messages')),
+ Output('texts', dict, doc=_('Dict of I18N messages')),
)
def execute(self, **options):
- return dict([("messages",json_serialize(self.messages))])
+ return dict(texts=json_serialize(self.messages))
def output_for_cli(self, textui, result, *args, **options):
print json.dumps(result, default=json_serialize)
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
