millerf/django-channels-jsonrpc

Add JSON-RPC ability to channels

django-channels-jsonrpc's Issues

Move all encoding/decoding/validating logic to a dependency package

We could use this package http://json-rpc.readthedocs.io/en/latest/index.html (for example) as a dependency and get rid of all protocol-agnostic questions (encoding/decoding/validation). We will not have to think wheater our JSON-RPC is correct (according to the Spec) or not.

Improve outbound notifications

While using new outbound notifications in my project, I realized that our current implementation is not a completed solution:
JsonRpcConsumer.notify_group()'s "params" parameter is directly assigned to JSON-RPC "params" without any checks, that may lead to incorrect JSON output. "params" must be forced to list or dict types, at least. But using python's argument unpacking and named parameters we could make it in a better way.
In fact, we a making a JsonRpc client (asuming outbound calls), which is aimed to work ot top of Django Channels. So I think we shoud go the same way as most clients do, using Channels as an endpoint. A standart approach of all clients looks like this:

client = JsonRpcClient(endpoint) #  endpoint = some http url or websocket url
client.call(method_name, param1, param2)
client.notification(method_name, param1, param2)

In our case it might look the same way, for example:

JsonRpcClient(Group('name')).notify(method, param1, param2) # notification with positional arguments as per JSON-RPC spec
JsonRpcClient(message.reply_channel).notify(method, param1=value1, param2=value2) # notification with named arguments

So the solution is:

1. We introduce a new class JsonRpcClient along with our main JsonRpcConsumer.
2. We use Channels "Group" and "reply_channel" objects as possible endpoints.
3. JsonRpcClient has one method - "notify", which sends outbound notifications. We can not impelment "call" method we will not be able to retrieve response anyway.
4. When notify is called with positional arguments - we encode them as a list, and when with named - as a dict. This way we ensure that arguments are passed correctly without any type-forcing. We just need to check that user did not pass both positional and named parameters together.

I may implement it tomorrow and make a pull request, it you like this approach.

Access to original_message is not thread-safe

This code is not thread-safe:

global original_message
original_message = original_msg
result = method(*args, **kwargs)
original_message = None

If you have two JsonRpcWebsocketConsumer you can get a moment, when one thread reset original_message and other tries to read it.

Notifications support

It would be great to implement inbound (and outbound) notifications.
I could make a pull-request. I think inbound notifications may be processed by the same __process method (but not return any value, so nothing will be sent back) and handlers may be defined with the same JsonRpcWebsocketConsumer.rpc_method() decorator... what do you think?

By outbound notifications i mean possibility to send JsonRpc notifications to "reply_channel" or to Groups (make implementation bi-directional in some way). I think a good starting point would be to create some helper method like "create_notification(method, params)" that just takes method name and params and returnes a correct JSON structure defined in JsonRpc protocol, so that it may be used like this:

original_message.reply_channel.send({"text": create_notification(method, params)})
Group("xxx").send({"text": create_notification(method, params)})

Dead Link http://channels.readthedocs.io/en/stable/generics.html#websockets

Hej Guys!
You did a nice work with that jsonrpcconsumer... Just one question. I tried to integrate your package into my already build up project and following your documentation i realized that the link to channels is dead.
Further i realized that your requirements need channels==1.1.6 whilst the latest stable version is around 2.1.1.
So my question, did you already tested agains channels>=2.1.0

kine regards, Mr. J

Implement JsonRpcHttpConsumer?

Hi.

What about implementing another consumer, for http requests in the same fashion as websocket?

Why threaded processing?

What is the reason for executing JsonRpc in separate threads?

typo: websockets instead websocket

django-channels-jsonrpc/channels_jsonrpc/jsonrpcconsumer.py

Line 14 in b71bc9e

from channels.generic.websockets import WebsocketConsumer