Added service multiplexing support. #88
Conversation
|
I checked the build failures and they're in unrelated code. Confirmed tests pass for these changes and that it passes PEP8. |
|
@tmehlinger it's related to recent PEP8 changes. The develop branch was upgraded to latest version just now. Can you please try rebase the branch on develop and fix the pep8 related problem? |
| # convert kwargs to args | ||
| api_args = [args.thrift_spec[k][1] | ||
| for k in sorted(args.thrift_spec)] | ||
| call = lambda: getattr(self._handler, api)( |
There was a problem hiding this comment.
There's also a E731 "do not assign a lambda expression, use a def" violation in here.
I've updated the code in develop branch to use the def.
|
👍 LGTM, I'll merge it once the ci passed. |
|
Sure thing, I'll have it in shortly. |
This introduces a `TMultiplexingProcessor` class which extends `TProcessor`. This adds the ability for developers to develop services that implement multiple Thrift interfaces. It is inspired by the `TMultiplexedProcessor` from the Thrift Java library.
|
Done. It failed to clone the repository on one of the test envs but everything else looks good. |
Added service multiplexing support.
|
Yah, sometimes travis ci do have some of the envs failed. I triggered re-run and it passed now. |
|
👍 thanks! |
|
Hi, I am curious that if there is any reason that one service can be only registered once in one server? I saw that service registration is done by adding functions to a dict, and one service can be only registered once, and requires no Multiplexing in client protocol. This is different from the solution in the original apache issue: https://issues.apache.org/jira/browse/THRIFT-563, which basically allow registering multiple instances of same service using different name. Is there any plan to support the latter flavor? |
|
@xvblack yah you're right, the implementation is different. I'd also prefer the upstream implementation to stay compatible with Apache Thrift. @tmehlinger how do you think about this idea? The Apache solution is a little more complex but actually avoided the |
|
@lxyu I think it's a great idea. I hadn't actually thought of a good way to resolve the name collision so I took the easy way out. An upstream-compatible solution is going to be way better than what I slapped together. |
This is a re-do of #79.
This introduces a TMultiplexingProcessor class which extends TProcessor. This adds the ability for developers to develop services that implement multiple Thrift interfaces. It is inspired by the TMultiplexedProcessor from the Thrift Java library.
Example here: http://stackoverflow.com/questions/19614648/service-multiplexing-using-apache-thrift