Re: [Freeipa-devel] [PATCH] 0119 Switch client to JSON-RPC
On 11/26/2013 03:06 PM, Jan Cholasta wrote:
On 18.10.2013 12:26, Petr Viktorin wrote:
On 10/17/2013 06:08 PM, Jan Cholasta wrote:
Hi,
On 7.10.2013 18:16, Petr Viktorin wrote:
On 08/12/2013 10:17 AM, Petr Viktorin wrote:
On 08/02/2013 11:13 AM, Petr Viktorin wrote:
On 05/10/2013 04:54 PM, Petr Viktorin wrote:
On 04/01/2013 11:37 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/15/2013 12:36 PM, Petr Viktorin wrote:
I meant to hold this patch a while longer to let it mature, but
from
what Brian Smith asked on the user list it seems it could help
him.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
See the design page for what the patch does.
As much as I've tried to avoid them, the code includes some
workarounds:
It extends xmlrpclib to also support JSON. This is rather
intrusive,
but
to not do that I'd need to write a parallel stack for JSON,
without
the
help of a standard library.
It would be nice to write the JSON stack in the future anyway, this
indeed seems intrusive to me.
Yes, it would.
The registration of either jsonclient or xmlclient as
rpcclient in
the
API also needs a bit of magic, since the framework requires the
class
name to match the attribute.
You could also put the name of the plugin in a configuration option and
address the plugin like api.Backend[api.env.rpcclient_plugin], but I
guess your solution is no worse.
To prevent backwards compatibility problems, we need to ensure
that
all
official JSON clients send the API version, so this patch
should be
applied after my patches 0104-0106.
Updating to current master.
Please reverse this change in ipalib/rpc.py:
@@ -665,8 +788,6 @@ class xmlclient(Connectible):
except Exception, e:
if not fallback:
raise
-else:
-self.log.info('Connection to %s failed with
%s',
url, e)
serverproxy = None
This logs connection errors when the client fails over to another
server.
Thanks. Done, and rebased to master.
Thanks for the patch, it works for me.
I have just a few nitpicks:
def forward(self, *args, **kw):
-Forward call over XML-RPC to this same command on server.
+Forward call over JSON-RPC to this same command on server.
The new docstring is not entirely true.
Fixed, thanks
+def send_content(self, connection, request_body):
+if self.protocol == 'json':
+connection.putheader(Content-Type, application/json)
+else:
+connection.putheader(Content-Type, text/xml)
+
+connection.putheader(Content-Length, str(len(request_body)))
+connection.endheaders(request_body)
The original implementation of send_content in the Transport class sets
up gzip compression. I think it may be useful to do it here as well.
We don't opt in for gzip compression, so that's unnecessary.
I've added a comment.
+def rpc_uri_from_env(self, env):
+raise NotImplementedError('RPCClient.rpc_uri_from_env')
...
-xmlrpc_uri = self.env.xmlrpc_uri
+rpc_uri = self.rpc_uri_from_env(self.env)
I don't think this is necessary, since Env is also a mapping, you can do
this instead:
+env_rpc_uri_var = None
...
-xmlrpc_uri = self.env.xmlrpc_uri
+rpc_uri = self.env[env_rpc_uri_var]
You're right, changed
+class xmlclient(RPCClient):
+session_path = '/ipa/session/xml'
+server_proxy_class = ServerProxy
+protocol = 'xml'
+wrap_functions = xml_wrap, xml_unwrap
...
+class jsonclient(RPCClient):
+session_path = '/ipa/session/json'
+server_proxy_class = JSONServerProxy
+protocol = 'json'
+wrap_functions = identity, identity
It seems to me that json_encode_binary and json_decode_binary are
equivallent to xml_wrap and xml_unwrap, is there a reason you used the
identity function in jsonclient.wrap_functions?
Yes, it's for unwrapping error results. For JSON, we want the decoding
done before the error is raised, but for XML no decoding is done (error
results can't contain binary data).
I've moved this into a single method, hopefully it's clearer this way.
Thanks. ACK once you remove the unused identity function.
Thank you!
Removed the function and pushed to master:
73b8047b2298d347475a5c8d9f1853052ddced57
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0119 Switch client to JSON-RPC
Hi,
On 7.10.2013 18:16, Petr Viktorin wrote:
On 08/12/2013 10:17 AM, Petr Viktorin wrote:
On 08/02/2013 11:13 AM, Petr Viktorin wrote:
On 05/10/2013 04:54 PM, Petr Viktorin wrote:
On 04/01/2013 11:37 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/15/2013 12:36 PM, Petr Viktorin wrote:
I meant to hold this patch a while longer to let it mature, but from
what Brian Smith asked on the user list it seems it could help him.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
See the design page for what the patch does.
As much as I've tried to avoid them, the code includes some
workarounds:
It extends xmlrpclib to also support JSON. This is rather intrusive,
but
to not do that I'd need to write a parallel stack for JSON, without
the
help of a standard library.
It would be nice to write the JSON stack in the future anyway, this
indeed seems intrusive to me.
The registration of either jsonclient or xmlclient as rpcclient in
the
API also needs a bit of magic, since the framework requires the
class
name to match the attribute.
You could also put the name of the plugin in a configuration option and
address the plugin like api.Backend[api.env.rpcclient_plugin], but I
guess your solution is no worse.
To prevent backwards compatibility problems, we need to ensure that
all
official JSON clients send the API version, so this patch should be
applied after my patches 0104-0106.
Updating to current master.
Please reverse this change in ipalib/rpc.py:
@@ -665,8 +788,6 @@ class xmlclient(Connectible):
except Exception, e:
if not fallback:
raise
-else:
-self.log.info('Connection to %s failed with %s',
url, e)
serverproxy = None
This logs connection errors when the client fails over to another
server.
Thanks. Done, and rebased to master.
Thanks for the patch, it works for me.
I have just a few nitpicks:
def forward(self, *args, **kw):
-Forward call over XML-RPC to this same command on server.
+Forward call over JSON-RPC to this same command on server.
The new docstring is not entirely true.
+def send_content(self, connection, request_body):
+if self.protocol == 'json':
+connection.putheader(Content-Type, application/json)
+else:
+connection.putheader(Content-Type, text/xml)
+
+connection.putheader(Content-Length, str(len(request_body)))
+connection.endheaders(request_body)
The original implementation of send_content in the Transport class sets
up gzip compression. I think it may be useful to do it here as well.
+def rpc_uri_from_env(self, env):
+raise NotImplementedError('RPCClient.rpc_uri_from_env')
...
-xmlrpc_uri = self.env.xmlrpc_uri
+rpc_uri = self.rpc_uri_from_env(self.env)
I don't think this is necessary, since Env is also a mapping, you can do
this instead:
+env_rpc_uri_var = None
...
-xmlrpc_uri = self.env.xmlrpc_uri
+rpc_uri = self.env[env_rpc_uri_var]
+class xmlclient(RPCClient):
+session_path = '/ipa/session/xml'
+server_proxy_class = ServerProxy
+protocol = 'xml'
+wrap_functions = xml_wrap, xml_unwrap
...
+class jsonclient(RPCClient):
+session_path = '/ipa/session/json'
+server_proxy_class = JSONServerProxy
+protocol = 'json'
+wrap_functions = identity, identity
It seems to me that json_encode_binary and json_decode_binary are
equivallent to xml_wrap and xml_unwrap, is there a reason you used the
identity function in jsonclient.wrap_functions?
The changes look really good. The show stopper is that using jsonrpc
doesn't create a session key. I noticed that xmlrpc_uri is hardcoded
into ipalib/session.py but it appears the issue is deeper than that.
That uses only the hostname from xmlrpc_uri. When using different
hostnames in xmlrpc_uri and jsonrpc_uri *on the server*, it'll put the
wrong domain in the cookie. In this case I think it's the configuration
that's wrong.
The real problem is that KerberossSession code which creates the
cookie,
was not called by the JSON server.
Patch 0223 adds it.
ACK.
Patch 0224 adds the server class name (e.g. [jsonserver_kerb]) to the
server logs. It should help debug problems specific to a
backend/protocol.
ACK.
Rebased onto current master.
Another minor rebase
Rebased again.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0119 Switch client to JSON-RPC
Petr Viktorin wrote:
On 01/15/2013 12:36 PM, Petr Viktorin wrote:
I meant to hold this patch a while longer to let it mature, but from
what Brian Smith asked on the user list it seems it could help him.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
See the design page for what the patch does.
As much as I've tried to avoid them, the code includes some workarounds:
It extends xmlrpclib to also support JSON. This is rather intrusive, but
to not do that I'd need to write a parallel stack for JSON, without the
help of a standard library.
The registration of either jsonclient or xmlclient as rpcclient in the
API also needs a bit of magic, since the framework requires the class
name to match the attribute.
To prevent backwards compatibility problems, we need to ensure that all
official JSON clients send the API version, so this patch should be
applied after my patches 0104-0106.
Updating to current master.
Please reverse this change in ipalib/rpc.py:
@@ -665,8 +788,6 @@ class xmlclient(Connectible):
except Exception, e:
if not fallback:
raise
-else:
-self.log.info('Connection to %s failed with %s',
url, e)
serverproxy = None
This logs connection errors when the client fails over to another server.
The changes look really good. The show stopper is that using jsonrpc
doesn't create a session key. I noticed that xmlrpc_uri is hardcoded
into ipalib/session.py but it appears the issue is deeper than that.
I did some basic testing with an old client against this server and
things seem to be fine.
I get the same results running the unit tests with both rpclib settings.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0119 Switch client to JSON-RPC
On 01/15/2013 12:36 PM, Petr Viktorin wrote:
I meant to hold this patch a while longer to let it mature, but from
what Brian Smith asked on the user list it seems it could help him.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
See the design page for what the patch does.
As much as I've tried to avoid them, the code includes some workarounds:
It extends xmlrpclib to also support JSON. This is rather intrusive, but
to not do that I'd need to write a parallel stack for JSON, without the
help of a standard library.
The registration of either jsonclient or xmlclient as rpcclient in the
API also needs a bit of magic, since the framework requires the class
name to match the attribute.
To prevent backwards compatibility problems, we need to ensure that all
official JSON clients send the API version, so this patch should be
applied after my patches 0104-0106.
Updating to current master.
--
Petr³
From 87bade151fd8865c29a16d817c841074754a0d26 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 19 Dec 2012 04:25:24 -0500
Subject: [PATCH] Switch client to JSON-RPC
Modify ipalib.rpc to support JSON-RPC in addition to XML-RPC.
This is done by subclassing and extending xmlrpclib, because
our existing code relies on xmlrpclib internals.
The URI to use is given in the new jsonrpc_uri env variable. When
it is not given, it is generated from xmlrpc_uri by replacing
/xml with /json.
The rpc_json_uri env variable existed before, but was unused,
undocumented and not set the install scripts.
This patch removes it in favor of jsonrpc_uri (for consistency
with xmlrpc_uri).
Add the rpc_protocol env variable to control the protocol
IPA uses. rpc_protocol defaults to 'jsonrpc', but may be changed
to 'xmlrpc'.
Make backend.Executioner and tests use the backend specified by
rpc_protocol.
For compatibility with unwrap_xml, decoding JSON now gives tuples
instead of lists.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
---
ipa-client/man/default.conf.5 |8 +-
ipalib/backend.py |2 +-
ipalib/config.py | 21 ++-
ipalib/constants.py |7 +-
ipalib/frontend.py|4 +-
ipalib/plugins/{xmlclient.py = rpcclient.py} | 24 ++-
ipalib/rpc.py | 271 +
ipaserver/rpcserver.py| 113 +-
tests/test_cmdline/test_cli.py|4 +-
tests/test_ipaserver/test_rpcserver.py|4 +-
tests/test_xmlrpc/test_dns_plugin.py |4 +-
tests/test_xmlrpc/test_trust_plugin.py|4 +-
tests/test_xmlrpc/xmlrpc_test.py |8 +-
13 files changed, 312 insertions(+), 162 deletions(-)
rename ipalib/plugins/{xmlclient.py = rpcclient.py} (58%)
diff --git a/ipa-client/man/default.conf.5 b/ipa-client/man/default.conf.5
index a0804e39f946b3202acc32cc6a7ccbdb8f2626e6..edaa16ee15cdf74caa19910854e5abcabbd525de 100644
--- a/ipa-client/man/default.conf.5
+++ b/ipa-client/man/default.conf.5
@@ -179,7 +179,13 @@ Used internally in the IPA source package to verify that the API has not changed
When True provides more information. Specifically this sets the global log level to info.
.TP
.B xmlrpc_uri URI
-Specifies the URI of the XML\-RPC server for a client. This is used by IPA and some external tools as well, such as ipa\-getcert. e.g. https://ipa.example.com/ipa/xml
+Specifies the URI of the XML\-RPC server for a client. This may be used by IPA, and is used by some external tools, such as ipa\-getcert. Example: https://ipa.example.com/ipa/xml
+.TP
+.B jsonrpc_uri URI
+Specifies the URI of the JSON server for a client. This is used by IPA. If not given, it is derived from xmlrpc_uri. Example: https://ipa.example.com/ipa/json
+.TP
+.B rpc_protocol URI
+Specifies the type of RPC calls IPA makes: 'jsonrpc' or 'xmlrpc'. Defaults to 'jsonrpc'.
.TP
The following define the containers for the IPA server. Containers define where in the DIT that objects can be found. The full location is the value of container + basedn.
container_accounts: cn=accounts
diff --git a/ipalib/backend.py b/ipalib/backend.py
index 7be38ecc80faf03e735813fb1e2d0eba5c347800..b94264236795b65905ba425ef15e452e7a66625b 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -113,7 +113,7 @@ class Executioner(Backend):
if self.env.in_server:
self.Backend.ldap2.connect(ccache=ccache)
else:
-self.Backend.xmlclient.connect(verbose=(self.env.verbose = 2),
+self.Backend.rpcclient.connect(verbose=(self.env.verbose = 2),
fallback=self.env.fallback, delegate=self.env.delegate)
if client_ip is not None:
setattr(context, client_ip, client_ip)
diff --git a/ipalib/config.py b/ipalib/config.py
index
[Freeipa-devel] [PATCH] 0119 Switch client to JSON-RPC
I meant to hold this patch a while longer to let it mature, but from
what Brian Smith asked on the user list it seems it could help him.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
See the design page for what the patch does.
As much as I've tried to avoid them, the code includes some workarounds:
It extends xmlrpclib to also support JSON. This is rather intrusive, but
to not do that I'd need to write a parallel stack for JSON, without the
help of a standard library.
The registration of either jsonclient or xmlclient as rpcclient in the
API also needs a bit of magic, since the framework requires the class
name to match the attribute.
To prevent backwards compatibility problems, we need to ensure that all
official JSON clients send the API version, so this patch should be
applied after my patches 0104-0106.
--
Petr³
From 14400d8561560fc4dac7cd026d3ae5e8832c97f8 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 19 Dec 2012 04:25:24 -0500
Subject: [PATCH] Switch client to JSON-RPC
Modify ipalib.rpc to support JSON-RPC in addition to XML-RPC.
This is done by subclassing and extending xmlrpclib, because
our existing code relies on xmlrpclib internals.
The URI to use is given in the new jsonrpc_uri env variable. When
it is not given, it is generated from xmlrpc_uri by replacing
/xml with /json.
The rpc_json_uri env variable existed before, but was unused,
undocumented and not set the install scripts.
This patch removes it in favor of jsonrpc_uri (for consistency
with xmlrpc_uri).
Add the rpc_protocol env variable to control the protocol
IPA uses. rpc_protocol defaults to 'jsonrpc', but may be changed
to 'xmlrpc'.
Make backend.Executioner and tests use the backend specified by
rpc_protocol.
For compatibility with unwrap_xml, decoding JSON now gives tuples
instead of lists.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
---
ipa-client/man/default.conf.5 |8 +-
ipalib/backend.py |2 +-
ipalib/config.py | 21 ++-
ipalib/constants.py |7 +-
ipalib/frontend.py|4 +-
ipalib/plugins/{xmlclient.py = rpcclient.py} | 24 ++-
ipalib/rpc.py | 271 +
ipaserver/rpcserver.py| 113 +-
tests/test_cmdline/test_cli.py|4 +-
tests/test_ipaserver/test_rpcserver.py|4 +-
tests/test_xmlrpc/test_dns_plugin.py |4 +-
tests/test_xmlrpc/xmlrpc_test.py |8 +-
12 files changed, 310 insertions(+), 160 deletions(-)
rename ipalib/plugins/{xmlclient.py = rpcclient.py} (58%)
diff --git a/ipa-client/man/default.conf.5 b/ipa-client/man/default.conf.5
index a0804e39f946b3202acc32cc6a7ccbdb8f2626e6..edaa16ee15cdf74caa19910854e5abcabbd525de 100644
--- a/ipa-client/man/default.conf.5
+++ b/ipa-client/man/default.conf.5
@@ -179,7 +179,13 @@ Used internally in the IPA source package to verify that the API has not changed
When True provides more information. Specifically this sets the global log level to info.
.TP
.B xmlrpc_uri URI
-Specifies the URI of the XML\-RPC server for a client. This is used by IPA and some external tools as well, such as ipa\-getcert. e.g. https://ipa.example.com/ipa/xml
+Specifies the URI of the XML\-RPC server for a client. This may be used by IPA, and is used by some external tools, such as ipa\-getcert. Example: https://ipa.example.com/ipa/xml
+.TP
+.B jsonrpc_uri URI
+Specifies the URI of the JSON server for a client. This is used by IPA. If not given, it is derived from xmlrpc_uri. Example: https://ipa.example.com/ipa/json
+.TP
+.B rpc_protocol URI
+Specifies the type of RPC calls IPA makes: 'jsonrpc' or 'xmlrpc'. Defaults to 'jsonrpc'.
.TP
The following define the containers for the IPA server. Containers define where in the DIT that objects can be found. The full location is the value of container + basedn.
container_accounts: cn=accounts
diff --git a/ipalib/backend.py b/ipalib/backend.py
index 7be38ecc80faf03e735813fb1e2d0eba5c347800..b94264236795b65905ba425ef15e452e7a66625b 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -113,7 +113,7 @@ class Executioner(Backend):
if self.env.in_server:
self.Backend.ldap2.connect(ccache=ccache)
else:
-self.Backend.xmlclient.connect(verbose=(self.env.verbose = 2),
+self.Backend.rpcclient.connect(verbose=(self.env.verbose = 2),
fallback=self.env.fallback, delegate=self.env.delegate)
if client_ip is not None:
setattr(context, client_ip, client_ip)
diff --git a/ipalib/config.py b/ipalib/config.py
index 3c9aeaa2880cb67c7f230ecba4839d37e77b04f2..f86c0a5ea3885d2bf89712f91b0c0705dceedd32 100644
--- a/ipalib/config.py
+++
