feat: add an option to enable DirectPath xDS #1968
Conversation
gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java
Outdated
Show resolved
Hide resolved
mohanli-ml
changed the title
|
@suztomo Hi Tomo, can you review this PR? Thanks! |
|
I assigned Blake. |
| return true; | ||
| } | ||
| // Method 2: Enable DirectPath xDS by option. | ||
| if (Boolean.TRUE.equals(useDirectPathXds)) { |
There was a problem hiding this comment.
Usually an explicitly set value has higher priority than environment variable, otherwise the users of this public field could be confused why their explicitly set value does not work.
That being said, this class and the whole DirectPath feature is supposed to be use by internal users only, so I'm fine with it if it's a requirement that env takes precedence over options, at least we should document this behavior in Javadoc.
There was a problem hiding this comment.
I reversed the order and check the option first.
| @@ -684,6 +702,13 @@ public Builder setAllowNonDefaultServiceAccount(boolean allowNonDefaultServiceAc | |||
| return this; | |||
| } | |||
|
|
|||
| /** Use DirectPath xDS. Only valid if DirectPath is attempted. */ | |||
There was a problem hiding this comment.
Document that an environment variable could override this option.
There was a problem hiding this comment.
The new API is used to enable DirectPath xDS, but not disable. So I removed the bool parameter. In this way, the environment can not override the option.
| @@ -262,6 +264,20 @@ private boolean isDirectPathEnabled(String serviceAddress) { | |||
| return false; | |||
| } | |||
|
|
|||
| private boolean isDirectPathXdsUsed() { | |||
There was a problem hiding this comment.
Can we test this behavior in unit tests?
|
@mohanli-ml, we recently moved this repo to gapic-generator-java, can you please close this PR and open a new PR in gapic-generator-java? |
|
(I didn't notice this PR was created in an old repo. Thanks.) |
|
SonarCloud Quality Gate failed. |
Currently DirectPath xDS is enabled by using env. We want to also provide an option for this. Java PR: googleapis/gax-java#1968
|
@blakeli0 The new PR: googleapis/sdk-platform-java#1643. |
Currently DirectPath xDS is enabled by using env. We want to also provide an option for this.
Go PR: googleapis/google-api-go-client#1942
@veblush