Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Add setAttemptDirectPath(boolean) for InstantiatingGrpcChannelProvider.Builder #1015

Merged
merged 5 commits into from May 28, 2020

Conversation

WeiranFang
Copy link
Contributor

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

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 6, 2020
@codecov
Copy link

codecov bot commented Apr 6, 2020

Codecov Report

Merging #1015 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             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     
Impacted Files Coverage Δ Complexity Δ
...api/gax/grpc/InstantiatingGrpcChannelProvider.java 78.75% <100.00%> (-1.14%) 33.00 <2.00> (-2.00)
...le/api/gax/httpjson/HttpJsonExceptionCallable.java 79.48% <0.00%> (-2.10%) 2.00% <0.00%> (ø%)
...java/com/google/api/gax/rpc/UnaryCallSettings.java 100.00% <0.00%> (ø) 6.00% <0.00%> (+1.00%)
.../java/com/google/api/gax/batching/BatcherImpl.java 98.03% <0.00%> (+0.03%) 16.00% <0.00%> (ø%)
...le/api/gax/retrying/ExponentialRetryAlgorithm.java 95.83% <0.00%> (+0.08%) 13.00% <0.00%> (ø%)
.../google/api/gax/batching/BatchingCallSettings.java 96.87% <0.00%> (+0.57%) 6.00% <0.00%> (+1.00%)
...a/com/google/api/gax/rpc/BatchingCallSettings.java 95.00% <0.00%> (+0.71%) 8.00% <0.00%> (+1.00%)
.../com/google/api/gax/httpjson/HttpHeadersUtils.java 85.71% <0.00%> (+1.09%) 11.00% <0.00%> (ø%)
...oogle/api/gax/rpc/ServerStreamingCallSettings.java 70.49% <0.00%> (+3.21%) 7.00% <0.00%> (+1.00%)

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 909bf1b...0a36964. Read the comment docs.

// 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))
Copy link
Contributor

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

Copy link
Contributor Author

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..

Copy link
Contributor

@igorbernstein2 igorbernstein2 left a 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

@WeiranFang
Copy link
Contributor Author

Adding @chingor13. Jeff can you take a look at this PR? Thanks!

@chingor13 chingor13 closed this May 27, 2020
@chingor13 chingor13 reopened this May 27, 2020
@chingor13 chingor13 merged commit acf1fec into googleapis:master May 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants