This repository has been archived by the owner on Sep 26, 2023. It is now read-only.
feat: update DirectPath environment variables #1412
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
google-cla
bot
added
the
cla: yes
elharo
suggested changes
Jun 29, 2021
| // Only check attemptDirectPath when DIRECT_PATH_ENV_DISABLE_DIRECT_PATH is not set. | ||
| if (Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))) { | ||
| return false; | ||
| } | ||
| if (attemptDirectPath != null) { | ||
| return attemptDirectPath; | ||
| } | ||
| // Only check DIRECT_PATH_ENV_VAR when attemptDirectPath is not set. | ||
| String whiteList = envProvider.getenv(DIRECT_PATH_ENV_VAR); | ||
| if (whiteList == null) return false; |
There was a problem hiding this comment.
all ifs are multiline, per google style
| @@ -243,14 +244,22 @@ public ManagedChannel createSingleChannel() throws IOException { | |||
| return GrpcTransportChannel.create(outerChannel); | |||
| } | |||
|
|
|||
| private boolean isDirectPathEnabled() { | |||
| // TODO(weiranf): Use attemptDirectPath as the only indicator once setAttemptDirectPath is adapted | |||
There was a problem hiding this comment.
please file a bug for this and reference it here
There was a problem hiding this comment.
Created b/192398509 to myself.
| @@ -79,6 +79,7 @@ | |||
| */ | |||
| @InternalExtensionOnly | |||
| public final class InstantiatingGrpcChannelProvider implements TransportChannelProvider { | |||
| static final String DIRECT_PATH_ENV_VAR = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH"; | |||
There was a problem hiding this comment.
The new ones should be private, I agree, but for the old ones, I guess it will technically make it a breaknig change
There was a problem hiding this comment.
I made the new one private, and keep the old one as what it is (i.e., without adding private).
| @@ -246,6 +247,10 @@ public ManagedChannel createSingleChannel() throws IOException { | |||
| // TODO(weiranf): Use attemptDirectPath as the only indicator once setAttemptDirectPath is adapted | |||
| // and the env var is removed from client environment. | |||
| private boolean isDirectPathEnabled(String serviceAddress) { | |||
| // Only check attemptDirectPath when DIRECT_PATH_ENV_DISABLE_DIRECT_PATH is not set. | |||
| if (Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENV_DISABLE_DIRECT_PATH))) { | |||
There was a problem hiding this comment.
This code is hard to follow due to the double negative. Probably better broken up into 3 separate statements.
elharo
suggested changes
Jun 29, 2021
vam-google
approved these changes
Jun 29, 2021
| @@ -79,6 +79,7 @@ | |||
| */ | |||
| @InternalExtensionOnly | |||
| public final class InstantiatingGrpcChannelProvider implements TransportChannelProvider { | |||
| static final String DIRECT_PATH_ENV_VAR = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH"; | |||
There was a problem hiding this comment.
The new ones should be private, I agree, but for the old ones, I guess it will technically make it a breaknig change
elharo
approved these changes
Jun 30, 2021
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Jul 19, 2021
🤖 I have created a release \*beep\* \*boop\* --- ## [1.67.0](https://www.github.com/googleapis/gax-java/compare/v1.66.0...v1.67.0) (2021-07-19) ### Features * introduce closeAsync to Batcher ([#1423](https://www.github.com/googleapis/gax-java/issues/1423)) ([aab5288](https://www.github.com/googleapis/gax-java/commit/aab528803405c2b5f9fc89641f47abff948a876d)) * optimize unary callables to not wait for trailers ([#1356](https://www.github.com/googleapis/gax-java/issues/1356)) ([dd5f955](https://www.github.com/googleapis/gax-java/commit/dd5f955a3ab740c677fbc6f1247094798eb814a3)) * update DirectPath environment variables ([#1412](https://www.github.com/googleapis/gax-java/issues/1412)) ([4f63b61](https://www.github.com/googleapis/gax-java/commit/4f63b61f1259936aa4a1eaf9162218c787b92f2a)) ### Bug Fixes * remove `extends ApiMessage` from `HttpJsonStubCallableFactory` definition ([#1426](https://www.github.com/googleapis/gax-java/issues/1426)) ([87636a5](https://www.github.com/googleapis/gax-java/commit/87636a5812874a77e9004aab07607121efa43736)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
codyoss
pushed a commit
to googleapis/google-api-go-client
that referenced
this pull request
Feb 18, 2022
Add an environment variable GOOGLE_CLOUD_DISABLE_DIRECT_PATH to disable DirectPath. We expect users to use the EnableDirectPath option to control DirectPath for most cases, and this environment variable is for special cases like running both DP and GFE traffic on the same VM. As a result, only when this env variable is explicitly set to true will DirectPath be disabled. Same PR for Java: googleapis/gax-java#1412.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add GOOGLE_CLOUD_DISABLE_DIRECT_PATH to allow manually disable DirectPath. Currently in Bigtable and Spanner, DirectPath is attempted by default. We need a way to disable DirectPath for debugging purpose.