[gax-java] chore: generalize ExecutorService for transport layers #1098
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1098 +/- ##
============================================
+ Coverage 78.72% 78.74% +0.02%
- Complexity 1169 1191 +22
============================================
Files 204 204
Lines 5184 5213 +29
Branches 416 419 +3
============================================
+ Hits 4081 4105 +24
- Misses 928 932 +4
- Partials 175 176 +1
Continue to review full report at Codecov.
|
gax/src/main/java/com/google/api/gax/rpc/TransportChannelProvider.java
Outdated
Show resolved
Hide resolved
igorbernstein2
left a comment
There was a problem hiding this comment.
I think you need to some explicit typecasts to avoid calling the deprecated version of withExecutor. Please see my pr for examples
|
Done. |
gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
Outdated
Show resolved
Hide resolved
|
Note that the new method is a direct 1:1 replacement for the deprecated one, so semantically there shouldn't be coverage changes. |
igorbernstein2
left a comment
There was a problem hiding this comment.
LGTM, but someone from the gax team should approve
|
Thanks @igorbernstein2, could you please resolve the "changes requested" if no others are needed? Will wait until another gax reviewer approves. |
gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
Show resolved
Hide resolved
gax-grpc/src/test/java/com/google/api/gax/grpc/testing/LocalChannelProvider.java
Show resolved
Hide resolved
|
|
||
| /** @deprecated. Please use {@link #setExecutor(Executor)}. */ | ||
| @Deprecated | ||
| public Builder setExecutor(ScheduledExecutorService executor) { |
There was a problem hiding this comment.
This should be called setExecutorProvider and accept ExecutorProvider as an argument (i.e. put back the old method and deprecate it)
|
|
||
| /** @deprecated. Please use {@link #setExecutor(Executor)}. */ | ||
| @Deprecated | ||
| public Builder setExecutor(ScheduledExecutorService executor) { |
There was a problem hiding this comment.
This should be called setExecutorProvider and accept ExecutorProvider as an argument (i.e. put back the old method and deprecate it).
No description provided.