[WIP] Add client interceptors to channel factory #69
Conversation
This commit moves the `ApplicationContextBeanLookupUtils` into the 'internal' package and opens up its method to public visibility so that it can be shared amongst framework components. See spring-projects#52 Signed-off-by: Chris Bono <[email protected]>
This commit adds support for global and channel-specific client interceptors. See spring-projects#52 Signed-off-by: Chris Bono <[email protected]>
onobc
changed the title
| * blended with the global interceptors. | ||
| * @return a channel builder conifgured with the provided values | ||
| */ | ||
| ManagedChannelBuilder<?> createChannel(String authority, List<ClientInterceptor> interceptors, |
There was a problem hiding this comment.
I decided not doing a builder for just 1 more method. I think we will want to add one if we add 1 more method to this. Wdyt?
| @Bean | ||
| InProcessGrpcChannelFactory grpcChannelFactory() { | ||
| InProcessGrpcChannelFactory factory = new InProcessGrpcChannelFactory(); | ||
| @ConditionalOnMissingBean |
There was a problem hiding this comment.
I don't like that this bean is repeated here. If we want to pass one into the channel factory we need to create it here or split the client auto-config to include a @Configuration that both could import (this AC has a before condition on client AC).
| * | ||
| * @author Chris Bono | ||
| */ | ||
| public class ClientInterceptorsConfigurer implements InitializingBean { |
There was a problem hiding this comment.
So this is not like the other GrpcChannelConfigurer but I could not find a better name yet.
How do you feel about renaming the GrpcChannelConfigurer to GrpcChannelBuilderCustomizer like we did for the server customizers? Then this CIC would be the default ClientConfigurer? I am in favor of doing that.
|
|
||
| public DefaultGrpcChannelFactory(List<GrpcChannelConfigurer> configurers) { | ||
| public DefaultGrpcChannelFactory(List<GrpcChannelConfigurer> configurers, | ||
| ClientInterceptorsConfigurer interceptorsConfigurer) { |
There was a problem hiding this comment.
I added to <init> as it is required if user wants any kind of interceptors. I see that you did targets and credentials w/ setters. I am guessing to avoid constructor bloat and possible to let users override if need be? Should we do the same for this CIC?
|
Resubmitting PR in more logical commit order as things have changed a bit w/ the impl since this submission. |
1 participant
Note
This is a rough 1st pass to get buy-in before moving forward further
Commit 1: just moves the bean lookup util to the internal package so it can be shared w/ client package.
This commit adds support for global and channel-specific client interceptors.
See #52