Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with named parameters in the request.params #16

Closed
ryanhollister opened this issue Sep 26, 2011 · 9 comments · Fixed by #17
Closed

Issue with named parameters in the request.params #16

ryanhollister opened this issue Sep 26, 2011 · 9 comments · Fixed by #17

Comments

@ryanhollister
Copy link

Hello,

I believe there is a bug with how pyramid_rpc handles named parameters for a json-rpc calls. The 2.0 spec seems to indicate that it should be supported:

http://en.wikipedia.org/wiki/JSON-RPC#Version_2.0_.28Specification_Proposal.29

My POST request (from javascript) has the data:

{"jsonrpc":"2.0","method":"service_render","id":1,"params":{"test":"1"}}

The issue is that it seems the params is being cast to a tuple which causes it to loose its key/value association. I am still somewhat new to python so I wasn't able to pin point the correct solution, but I did track down the line where the data is being lost: jsonrpc.py line 126.

I would expect that when the "data" is passed to my method as such:

@jsonrpc_method(endpoint='api')
def service_render(request, data):
......

that "data" has its key/values preserved.

Thanks for the great work, I will continue to look at it and try and find a solution.

@ryanhollister
Copy link
Author

Nevermind, no more than 30 seconds after submitting this I realized that pyramid_rpc is taking the arguments a step further than I expected.

For example, if the request is:

{"jsonrpc":"2.0","method":"service_render","id":1,"params":{"test":"1","test2":"2"}}

then the method signature will need to be:

@jsonrpc_method(endpoint='api')
def service_render(request, data, data2):

and then pyramid_rpc will map "1" to data and "2" to data2.

This works, but is slightly unintuitive. I will propose some additional documentation around this.

Thank you and hopefully this issue will help someone else in the future.

@ryanhollister
Copy link
Author

I am making a mess of this issue now, but it still appears to not be working correctly. Even with my understanding from my comment above, the values for data and data2 are not the values of the named parameters but rather the name.

As I said the values were "1" and "2" for data and data2 respectively. This is not happening, the values for data and data2 are actually "test" and "test2"

Sorry for the confusion but I think I have identified the problem. Hopefully we can find a solution.

@ryanhollister
Copy link
Author

Ok, last post I promise :-)

So it turns out pyramid_rpc is SUPER smart.

It maps the named parameters to their associated parameter names... Meaning in order to call:

def service_render(request, data, data2):

the params should be:

{"jsonrpc":"2.0","method":"service_render","id":1,"params":{"data":"1","data2":"2"}}

Then the values are correct inside the method.

Sorry for the mess.

@mmerickel
Copy link
Member

Named parameters are not supported with pyramid_rpc right now. The fact that its working for you is lucky because the params dict is unordered.

I haven't seen a compelling reason to support named parameters. In all of the servers I've looked at while implementing JSON-RPC I have never seen one that supported it. If you know of one that does, let me know and I'll think about it. Same goes for clients. XML-RPC doesn't support them and gets along just fine because in reality it is RPC where you are calling a function with a defined signature, so it's on you (the client) to provide the arguments to match the signature in the correct order.

@ryanhollister
Copy link
Author

thank you for the follow up mmerickel.

Named parameters isn't that important to me. I think the only reason I ventured down that path was because I was getting inconsistent or unintended data being passed to my json-rpc exposed functions.

As I was saying before, I was getting the name of the parameters and not the values.

I would propose changing line 126 in jsonrpc.py from:

request.rpc_args = tuple(body.get('params', []))

to

request.rpc_args = tuple(body.get('params', []).values())

I very well may not understand the full repercussions of this change, but in my anecdotal scenario it fixed my issues.

@mmerickel
Copy link
Member

I will think about adding support for named parameters, but like I said: it's not supported right now. Thus, you should not be sending a dictionary as the params argument.

@epegzz
Copy link
Contributor

epegzz commented Sep 29, 2011

I spent about one hour today to figure out, why my named parameters sent by a java-jsonrpc-client do not make it to my rpc_method in pyramid. Actually i came here to have a look at the code, but then found this Issue here first.

Since named parameters are supported in the spec proposal ( http://en.wikipedia.org/wiki/JSON-RPC#Version_2.0_.28Specification_Proposal.29 ) I would love to see them supported by pyramid_rpc.

@epegzz
Copy link
Contributor

epegzz commented Sep 29, 2011

implemented in pull request #17

@mmerickel
Copy link
Member

The trunk now supports named parameters. This will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants