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
Update gRPC-Java to 1.30.0 #2814
Conversation
Motivation: gRPC-Java [1.30.0](https://github.com/grpc/grpc-java/releases/tag/v1.30.0) has been released We need a workaround for `ProtoReflectionService`. Because it does not implement `InternalNotifyOnServerBuild` anymore. See grpc/grpc-java#6967 line#2806 It is a temporary workaround until upstream handles grpc/grpc-java#7138. Motivations: - Upgrade gRPC to 1.30.0 from 1.29.0 - Add ProtoReflectionServiceInterceptor that injects dummy server to gRPC context. Result: You can run gRPC 1.30.0 with Armeria server. Fixes line#2806
grpc/src/main/java/com/linecorp/armeria/server/grpc/ProtoReflectionServiceInterceptor.java
Outdated
Show resolved
Hide resolved
grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java
Outdated
Show resolved
Hide resolved
static final ProtoReflectionServiceInterceptor INSTANCE = new ProtoReflectionServiceInterceptor(); | ||
|
||
@Nullable | ||
private static Server dummyServer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - is this supposed to be static? If so this interceptor can be static too, and yeah back to the idea we can call ProtoReflectionServiceInterceptor.addServerConfig()
where we used to call notifyOnBuild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replied at #2814 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick update!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @ikhoon !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If dummyServer
is static, wouldn't it make two different gRPC Server
instances provide the same dummyServer
instance? Is it OK even if the two Server
s have different gRPC services?
Ahh, I didn't think of that case. Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #2814 +/- ##
============================================
- Coverage 72.78% 72.68% -0.10%
- Complexity 11945 11947 +2
============================================
Files 1049 1048 -1
Lines 46554 46626 +72
Branches 5810 5829 +19
============================================
+ Hits 33883 33890 +7
- Misses 9705 9744 +39
- Partials 2966 2992 +26 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Motivation: gRPC-Java [1.30.0](https://github.com/grpc/grpc-java/releases/tag/v1.30.0) has been released We need a workaround for `ProtoReflectionService`. Because it does not implement `InternalNotifyOnServerBuild` anymore. See grpc/grpc-java#6967 line#2806 It is a temporary workaround until upstream handles grpc/grpc-java#7138. Modifications: - Upgrade gRPC to 1.30.0 from 1.29.0 - Add ProtoReflectionServiceInterceptor that injects dummy server to gRPC context. Result: You can run gRPC 1.30.0 with Armeria server. Fixes line#2806
Motivation:
gRPC-Java 1.30.0 has been released
We need a workaround for
ProtoReflectionService
. Because it does not implementInternalNotifyOnServerBuild
anymore.See grpc/grpc-java#6967 #2806
It is a temporary workaround until upstream handles grpc/grpc-java#7138.
Modifications:
gRPC context.
Result:
You can run gRPC 1.30.0 with Armeria server.
Fixes #2806