search

mbr / tinyrpc

A compact, modular transport and protocol agnostic RPC library. Does jsonrpc v2.
https://tinyrpc.readthedocs.org
MIT License
156 stars 53 forks source link

fix: always two-way issue #13

Closed satosi-k closed 7 years ago

satosi-k commented 10 years ago

A response is expected even if oneway=True is set to RPCClient#get_proxy() now. However, when oneway=True is set, there will be no response. This patch solves this issue.

satosi-k commented 10 years ago

I noticed that there is tinyrpc/tests directory. It seems that tests do not work well with my patch. Maybe I misunderstood some.

mbr commented 10 years ago

I'll leave this open until I have time to look into it more in-depth; I need to figure out exactly what feature you're missing here first =).

satosi-k commented 10 years ago

Thank you for the reply! =)

I am a contributor of Ryu. Ryu is an SDN Framework (In many cases, it is used as an OpenFlow controller). And tinyrpc is used in this project. Thank you for the great library. http://osrg.github.io/ryu/

We use WebSocket for transport and use JSON-RPC for protocol. We made transport class for WebSocket, JSONRPCProtocol was used as protocol as it is. https://github.com/osrg/ryu/blob/master/ryu/app/wsgi.py

And I thought that I wanted to make one-way notification to a server from a client. "One-way notification" is "Notification" in JSON-RPC 2.0. http://www.jsonrpc.org/specification#notification http://www.jsonrpc.org/specification#examples

It is written to specification as "The Server MUST NOT reply to a Notification". IOW, a client must not expect a reply. (It is maybe called "cast" in RPC terms)

I think that this function is included in ClientTransport#send_message(). When "expect_reply" of parameters is false, a reply is not expected. However, it does not work well.

It seems that it is caused by RPCClient#call(). This method expects a response always. IMHO, when one_way=True, it should work as "cast". And that behavior is wanted from me. My pull-request has intention of it.

I apologize for my broken English. Thanks.

lnoor commented 7 years ago

Hi @satosi-k I used your fix (though I didn't pull in your request). A new release with your fix will appear in a couple of weeks. Thanks for your contribution! (I've added your name to the list of contributors in the documentation)