-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Handler method to pass in client #454
Conversation
cc @tmc (seem to be attached to a lot of recent commits) |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
Thanks for your contribution! master has a build system fix, can you please rebase this? |
@tmc done, and I added a commit to regenerate the examples (which makes CI pass) |
@tmc this looks good to me. Feel free to rebase and merge |
so this is fantastic and is an obvious advantage over making the TCP call over loopback to achieve the same functionality - but may I bring up that using this bypasses grpc interceptors and stats handlers - which at least some of us use for authentication. (like this guy suggests: http://mycodesmells.com/post/authentication-in-grpc) Should we open another issue for how to support grpc constructs using this direct call method? Or, is this meant for those not using grpc featurettes like interceptors? |
@iluminae brings up a very good point. Could you at a minimum expand upon this downside in the documentation for this new method? |
@achew22 I've added some documentation clarifying, let me know if that isn't clear enough (and if not, how to change it ;) ) |
LGTM. once the tests pass let's merge this |
Right now if you want to embed the JSON proxy into your service you end up having to have a client talking to loopback which is messy. With this API addition you can simply pass in something which implements the client interface -- meaning you could wrap it yourself and return directly (without all the loopback and serialization overhead).
Forgot to regen the examples, just did-- this run should pass. |
* Add Handler method to pass in a client Right now if you want to embed the JSON proxy into your service you end up having to have a client talking to loopback which is messy. With this API addition you can simply pass in something which implements the client interface -- meaning you could wrap it yourself and return directly (without all the loopback and serialization overhead). Note that this does not apply interceptors and other gRPC niceties. Please ensure your application doesn't need interceptors before using this method.
Right now if you want to embed the JSON proxy into your service you end
up having to have a client talking to loopback which is messy. With this
API addition you can simply pass in something which implements the
client interface -- meaning you could wrap it yourself and return
directly (without all the loopback and serialization overhead).