Implemented JSONRPC 2.0 batch calls as per spec

Pylons / pyramid_rpc

RPC plugins for pyramid. XML-RPC, JSON-RPC, etc.

https://docs.pylonsproject.org/projects/pyramid-rpc/en/latest/

Other

27 stars 20 forks source link

Implemented JSONRPC 2.0 batch calls as per spec #28

Closed JustusW closed 9 years ago

JustusW commented 9 years ago

Implemented via Pyramid Tweens as suggested in TODO. All tests in test_jsonrpc.TestJSONRPCIntegration have been updated to work both batched and unbatched.

Testprotocol: (acpdev)juw@juw:~/Projects/pyramid_rpc/pyramid_rpc/tests$ python -m unittest test_jsonrpc ...No handlers could be found for logger "pyramid_rpc.jsonrpc"

.....................................

Ran 40 tests in 0.218s

mmerickel commented 9 years ago

Wow, good stuff! After a quick inspection this looks like a good approach. I'll try to review it more this weekend and get it merged.

JustusW commented 9 years ago

So, have you had time yet?

mmerickel commented 9 years ago

Have you put any thought into where this tween should show up in the ordering? For example, pyramid_tm is explicitly under the excview. I feel we may want the opposite?

https://github.com/Pylons/pyramid_tm/blob/master/pyramid_tm/__init__.py#L126

JustusW commented 9 years ago

Hello again, I've just added the over=EXCVIEW setting you were talking about. Sadly the 2.0 Specs are not totally explicit on this topic, but I believe it would be correct to return a separate response object even if a sub request fails. Is it possible to merge this sometime soon?

inklesspen commented 9 years ago

We really want this functionality at Axial; anything I can do to help get this in?

mmerickel commented 9 years ago

So I just spent a while bringing pyramid-rpc back into the modern world, fixing the test runner and docs stuff. Now I've finally got to dealing with this PR. I have several concerns.

1) Tests fail on py3. I've fixed most of these. 2) The tween affects every request coming into the pyramid app. This tween needs to find a way to affect only requests where the endpoint matches one of the registered endpoints to avoid running the json deserializer on every request. If this isn't clear we can always go into more detail. (registry.rpc_endpoints). 3) I'm fine with the empty string as a response probably but I'd like someone to double check with the spec and verify this behavior versus an empty array. I'd lean toward null instead of an empty array personally.

I pushed a new branch jsonrpc-batch-support that new PRs can be submitted against if anyone can help with these issues.