Skip to content

PHP: add call invoker#15749

Merged
ZhouyihaiDing merged 1 commit intogrpc:v1.13.xfrom
ZhouyihaiDing:call_invoker
Jun 27, 2018
Merged

PHP: add call invoker#15749
ZhouyihaiDing merged 1 commit intogrpc:v1.13.xfrom
ZhouyihaiDing:call_invoker

Conversation

@ZhouyihaiDing
Copy link
Copy Markdown
Contributor

@ZhouyihaiDing ZhouyihaiDing commented Jun 13, 2018

Hope this works as CallInvoker in C# does.
The original PR for C# is #5928
CallInvoker is described as to get rid of the tight coupling between MathClient and a "real" Channel. as well as allows intercepting of the calls.

It should work together with #15761.
I will add tests if this PR and #15761 are approved before merging.

The GCP extension will use ChannelInterface and CallInvoker to maintain a channel pool. The implementation and tests drafts can be seen at PR: add GCP extension support via call invoker

Hi @stanley-cheung , please review this API when you have time. Thanks!
This is the only way I find to hook a self-defined channel as well as intercept pre-process and post-process for each RPC naturally.
The purpose of the GCP extension is picking a channel from the pool inside call->start() and save/delete channel from the channel pool inside call->getStatus().

Code sample:

class MyCallInvoker implements Grpc\CallInvoker
{
    ...
}
$call_invoker = new MyCallInvoker();
$client = new Grpc\BaseStub('localhost:50051', [
    'credentials' => Grpc\ChannelCredentials::createInsecure(),
    'grpc_call_invoker' => $call_invoker,
]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

channelFactory or should it be createChannelFactory?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Can we ship it with this release or is it better to ship it later in 1.13.1?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do it in 1.13.0

@ZhouyihaiDing ZhouyihaiDing force-pushed the call_invoker branch 2 times, most recently from cc45fda to 30c852c Compare June 26, 2018 19:11
@ZhouyihaiDing ZhouyihaiDing changed the base branch from master to v1.13.x June 26, 2018 19:53
@ZhouyihaiDing ZhouyihaiDing force-pushed the call_invoker branch 2 times, most recently from 48e4fbf to 30c852c Compare June 26, 2018 20:13
@ZhouyihaiDing
Copy link
Copy Markdown
Contributor Author

Bazel Debug build for C/C++ - #15610
Bazel Opt build for C/C++ - #15610
There are two tests don't have logs: Bazel ASAN build for C/C++ and Bazel UBSAN build for C/C++. I think it doesn't caused by this PR. In addition, UBsan C and ASAN C are passed.

@ZhouyihaiDing ZhouyihaiDing merged commit 2f924c6 into grpc:v1.13.x Jun 27, 2018
@ZhouyihaiDing ZhouyihaiDing mentioned this pull request Jun 27, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants