Skip to content

Add temporary interceptor support#182

Merged
michaelbausor merged 15 commits intogoogleapis:masterfrom
michaelbausor:add-interceptor-temp-support
Jun 14, 2018
Merged

Add temporary interceptor support#182
michaelbausor merged 15 commits intogoogleapis:masterfrom
michaelbausor:add-interceptor-temp-support

Conversation

@michaelbausor
Copy link
Copy Markdown
Contributor

@michaelbausor michaelbausor commented Jun 1, 2018

Adds temporary support for an Interceptor-style interface in order to support logging of requests and responses via interceptors until interceptor support is available in gRPC.

This PR also adds a GapicInterceptor that can be used to implement common functionality such as metadata handling.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 6, 2018

Codecov Report

Merging #182 into master will decrease coverage by 0.39%.
The diff coverage is 54.28%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #182     +/-   ##
=========================================
- Coverage   84.79%   84.39%   -0.4%     
=========================================
  Files          56       58      +2     
  Lines        2756     2788     +32     
=========================================
+ Hits         2337     2353     +16     
- Misses        419      435     +16
Impacted Files Coverage Δ
src/ApiCore/Transport/Grpc/ForwardingUnaryCall.php 100% <100%> (ø)
src/ApiCore/Transport/Grpc/ForwardingCall.php 100% <100%> (ø)
src/ApiCore/Transport/GrpcTransport.php 54.45% <30.43%> (-8.51%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dcb47b...b293b47. Read the comment docs.

@michaelbausor michaelbausor changed the title WIP: Add temp interceptor support Add tempemporary interceptor support Jun 7, 2018
@michaelbausor michaelbausor changed the title Add tempemporary interceptor support Add temporary interceptor support Jun 7, 2018
@michaelbausor
Copy link
Copy Markdown
Contributor Author

@dwsupplee @jdpedrie @bshaffer if you have a chance, PTAL and let me know what you think.

Copy link
Copy Markdown
Contributor

@jdpedrie jdpedrie left a comment

Choose a reason for hiding this comment

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

in light of offline discussion, just a couple little things i noticed.

$argument,
array $metadata,
array $options,
$continuation

This comment was marked as spam.

This comment was marked as spam.

*
* @package Google\ApiCore\Transport\Grpc
*/
interface UnaryInterceptor

This comment was marked as spam.

This comment was marked as spam.

@michaelbausor michaelbausor mentioned this pull request Jun 13, 2018
Copy link
Copy Markdown
Contributor

@dwsupplee dwsupplee left a comment

Choose a reason for hiding this comment

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

Looks really good, just a few minor notes.

use Google\ApiCore\GrpcSupportTrait;
use Google\ApiCore\ServerStream;
use Google\ApiCore\ServiceAddressTrait;
use Google\ApiCore\Transport\Grpc\GapicInterceptorInterface;

This comment was marked as spam.

This comment was marked as spam.

*
* @type array $stubOpts Options used to construct the gRPC stub.
* @type Channel $channel Grpc channel to be used.
* @type UnaryInterceptorInterface[] $interceptors *INTERNAL* Interceptor support, required until

This comment was marked as spam.

This comment was marked as spam.

);
}

private function wrapExecuteWithInterceptor($execute, $interceptor)

This comment was marked as spam.

This comment was marked as spam.

@michaelbausor michaelbausor merged commit c25e384 into googleapis:master Jun 14, 2018
@michaelbausor michaelbausor deleted the add-interceptor-temp-support branch June 14, 2018 16:23
@michaelbausor michaelbausor mentioned this pull request Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants