explodinglabs / jsonrpcserver Goto Github PK

Just noticed this in batch requests:

# Call each request
response = [r.call(methods) for r in requests
            if not r.is_notification]

It looks like it's not calling notifications. Instead it should be removing the notification responses.

https://pythonhosted.org/pulsar/

Which provides the asynchronous versions of methods, Methods, and dispatch.

User can then import the same names either from jsonrpcserver or jsonrpcserver.async.

Continuing on from #10, Methods shouldn't subclass dict, because adding a function can trample on dict's attributes such as get.

Subclass object or list instead.

Give the option of a basic built-in server.

from jsonrpcserver import Methods
methods = Methods()
methods.add(lambda: 'meow', 'cat')
methods.serve_forever()

The examples in the readthedocs pages show the methods/handlers responding to all requests, however, jsonrpc servers should not respond to notification requests.

Incoming JSON-RPC requests have key names in camelCase.

Optionally, convert them to underscore before dispatching.

This will save a lot of headaches in the handling methods.

Just include the views.py. Use something like:

def post(request):
    response = dispatch(methods, request.body.decode("utf-8"))
    return HttpResponse(str(response), status=response.http_status)

Use pytest-asyncio

Subclass Methods with an AsyncMethods class, which starts it's own asynchronous server when you call serve_forever. (Requires #21 first)

Really enjoy using your library. I think you might need to update your ZeroMQ example though. In the example, you don't reply with a message if Response.is_notifcation == True. If you don't return a response, I believe this will never return a zmq.REP type socket to a readable state.

Subclass Methods from collections.MutableMapping rather than dict.

Gives the ability to validate any additions to the methods.

On rtd, including the api page

The jsonrpc spec for id in requests states An identifier established by the Client that MUST contain a String, Number, or NULL value if included. If it is not included it is assumed to be a notification.

Currently, jsonrpcserver throws when encountering "id":0. Possibly "id":"" should be allowed too. Handling the null value is less trivial.

https://github.com/bcb/jsonrpcserver/blob/master/jsonrpcserver/response.py#L80

        # Ensure we're not responding to a notification with data
        if not request_id:
            raise ValueError(
                'Requests must have an id, use NotificationResponse instead')

Get rid of the config module.

Instead, pass config settings to dispatch.

Currently with batch requests, it loops through each request validating each one against the schema. Validate the entire request in one go.

Add a config option to disable these

It can be preferable to turn the log entries off, particularly for large batch requests

http://aiozmq.readthedocs.io/

Logging requires too much boilerplate for the end user.

If the user hasn't configured logging, configure it for them.

Use either the websockets library, or aiohttp's websockets feature, or pulsar's. Or all of the above.

To enable notification errors, the documentation says incorrectly:

ErrorResponse.notification_errors = True

The correct code would be:

Request.notification_errors = True

The batch examples have already been added in batch-requests, but add the rest, in master.

Currently looping through the batch twice, once to build a list of Requests, and another time to call them. Do the Request part in the same time as calling. Basically remove the loop from Requests.__init__

Configuration currently requires modifying class attributes of Request and ErrorResponse. It should be easier, something like:

from jsonrpcserver import config
config.debug = True
config.schema_validation = False

There's no way to asynchronously process a request, which is a major inadequacy of the library. Should be able to await dispatch. Need a dispatch coroutine, or at least return a future from dispatch.

Will need some heavy refactoring to support both synchronous and asynchronous processing.

Hi, I have followed your instructions from https://github.com/bcb/jsonrpcserver/blob/master/examples/aiozmq_server.py

and it's ok.

But if I try to move

@methods.add
async def ping():
    return 'pong'

to another nearest file, jsonrpcserver can not find my ping() method.
What am I doing wrong?

Thank you in advance.

No need for the trailing underscore.

See #44

In the example ：https://jsonrpcserver.readthedocs.io/en/latest/examples.html#tornado

when client post a list json params, the server will be hang and do nothing.

☁  itiger  curl -X 'POST' -H 'Content-Type: application/json' -d '{"jsonrpc": "2.0", "method": "ping", "id": 1}' http://127.0.0.1:5000/
{"jsonrpc": "2.0", "result": "pong", "id": 1}%            
☁  itiger  curl -X 'POST' -H 'Content-Type: application/json' -d '[{"jsonrpc": "2.0", "method": "ping", "id": 1}, {"jsonrpc": "2.0", "method": "ping", "id": 2}]' http://127.0.0.1:5000/

the await methods.dispatch(request) will be hang , methods.dispatch return nothing.

For consistency

Would allow this usage:

methods.dispatch(request)

Saves having to import dispatch.

No need for it.

Rather than

methods = Methods()
@methods.add

It could just have an internal method registry, like:

from jsonrpcserver import methods

@methods.add
def ping():
    return 'pong'

if __name__ == '__main__':
    methods.serve_forever()

req = [Request('calc') for _ in range(10)]
await methods.dispatch(req)

Gives

TypeError: Object of type 'coroutine' is not JSON serializable

To minimise the payload, allow expanding serialised json before processing.

Such as avro, protobuf, hpack, gzip

In order to have simpler usage (#25), should reverse the order of the dispatch arguments, to make the "methods" argument optional. Without methods specified, it uses the internal methods registry.

dispatch(request)

A breaking change..........

This is a question as much as an issue. I have several jsonrpc methods that require db, redis, and other state/config access that would normally be passed using the the 'app' or 'request' object. What is the best method to access the parent http request and/or web app object from a method?

It seems like its necessary to modify/create/subclass:

• aio.methods
• async_methods.py
• async_dispatcher.dispatch
• AsyncRequest

I will be making these changes for use in our projects (jussi,yo,sbds,hivemind).

I'm thinking that changing the signature of the dispatch method (and all the other methods that lead to the jsonrpc method) to also accept *args, **kwargs is the best, most modular way:

async def handle(aio_http_request):
    statsd = aio_http_request.app.config['statsd_client']
    jsonrpc_request = await aio_http_request.text()
    response = await methods.dispatch(jsonrpc_request, aio_http_request, statsd=statsd)
    return web.json_response(response)

I presume the use case for db/cache/metrics isn't rare, so I thought I would solicit your expertise and opinion about how these changes are implemented, especially if you're interested in adding my changes upstream.

Thanks for you excellent work!