Add setAttemptDirectPath(boolean) for InstantiatingGrpcChannelProvider.Builder #1015
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1015 +/- ##
============================================
+ Coverage 78.66% 78.72% +0.05%
- Complexity 1167 1169 +2
============================================
Files 204 204
Lines 5152 5184 +32
Branches 413 416 +3
============================================
+ Hits 4053 4081 +28
- Misses 925 928 +3
- Partials 174 175 +1
Continue to review full report at Codecov.
|
gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
Show resolved
Hide resolved
gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
Outdated
Show resolved
Hide resolved
// TODO(weiranf): Add API in ComputeEngineCredentials to check default service account. | ||
// TODO(weiranf): Remove isDirectPathEnvVarEnabled once setAttemptDirectPath is adapted and the | ||
// env var is removed from client environment. | ||
if ((attemptDirectPath | isDirectPathEnvVarEnabled(serviceAddress)) |
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.
Please use ||
I think attemptDirectPath should be tristated:
- unset: use env var
- true: enable direct path
- false: disable direct path
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.
Nice catch! not sure what I was thinking..
gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
Outdated
Show resolved
Hide resolved
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.
LGTM, but might be good to have someone from the gax team approve as well
Adding @chingor13. Jeff can you take a look at this PR? Thanks! |
The current logic for deciding whether or not to attempt DirectPath is by checking the environment variable
GOOGLE_CLOUD_ENABLE_DIRECT_PATH
. However, we have decided to remove the dependency on this env variable and instead we should allow Yoshi library to attempt DirectPath by default.Added
setAttemptDirectPath()
so that Yoshi client library can use this API to set whether the client uses the DirectPath code path by default. The default value for attemptDirectPath is false.+cc @igorbernstein2