Open abhinav opened 8 years ago
@blampe @breerly @junchaowu @kriskowal @prashantv @malandrew
Server side API may need a little bit more thought. This API won't support per request state to be passed in. Like if you're implementing an HTTP service you may want to inject information from the raw HTTP request (headers, tracing information, etc.) into the endpoint handler.
Actually that brings up the same issue in the client API. We want users to be able to inject per request information from the call to the transport.
Looking good to me. One thing:
Seems like you want to always return concurrent.futures.Future
in the case of sync - any reason we can't do that?
@abhinav I'm not sure that we need to support request metadata in this library - after all, Apache Thrift suffers from the same issue. We'll be solving this in our RPC library.
Since we want thriftrw to better support HTTP (and presumably other transports that use Thrift message envelopes), let's get this discussion started.
Here's an inital API proposal for sync and async clients and servers.
For client-side support, we create a separate thriftrw-client Python package (probably a subdirectory under this project for now) that provides a
thriftrw_client
module with the following API:Where
transport
is any function that takes bytes to send the request and returns bytes of the response. This should suffice for synchronous clients and gevent based clients. We can provide default transport constructors likehttp_transport("http://.../thrift")
will usehttplib
or something.For asynchronous clients, we'll probably want to use
thriftrw_client.tornado
(and maybe laterthriftrw_client.asyncio
if needed) with the same interface excepttransport
returns a future instead of the response as-is. These can be extras in thethriftrw-client
's dependencies.Open question: A generic
Client
class like that will have to do a bunch of introspection of the Thrift module at runtime. Do we want to do something similar to TChannel'sclient_for
instead which generates theClient
class specific to that service in one go?For server-side, same story: A thriftrw-server package in a subdirectory with a
thriftrw_server
module that provides the following API:Same API for async servers except calling
dispatcher
returns a future. It'll plug into Tornado like:Open question: What to name Dispatcher?
Other things to consider while looking at this API:
TMultiplexedProtocol
that wraps the real protocol (binary/json/whatever) except that it changes the method name in the message envelope to "$serviceName:$methodName". This allows the server to dispatch to multiple Thrift services on the same endpoint. How will this API be affected if we wanted to support that? The reason I want us to consider this early on is that with multiple Thrift services in the same file, it's definitely possible and common to multiplex Thrift services.