Skip to content

Revert "feat: attemp DirectPath by default" #520

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 10, 2020

Conversation

WeiranFang
Copy link
Contributor

@WeiranFang WeiranFang commented Nov 10, 2020

Reverts #467

After this PR, gax-java relies on ComputeEngineCredentials to determine whether the client should use ComputeEngineChannelBuilder: https://github.com/googleapis/gax-java/blob/master/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java#L253

However, ComputeEngineCredentials has different assumptions than ComputeEngineChannelBuilder, which causes ComputeEngineChannelBuilder to fail on AppEngine: grpc/grpc-java#7604.

We need to rollback this PR, and make the fix in gax-java, and then roll-forward again.

@apolcyn
@mohanli-ml
@igorbernstein2
@mgarolera

@WeiranFang WeiranFang requested a review from a team as a code owner November 10, 2020 20:32
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable API. label Nov 10, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 10, 2020
@codecov
Copy link

codecov bot commented Nov 10, 2020

Codecov Report

Merging #520 (1abfa37) into master (0ef7c5d) will decrease coverage by 0.05%.
The diff coverage is 42.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #520      +/-   ##
============================================
- Coverage     81.32%   81.26%   -0.06%     
- Complexity     1128     1129       +1     
============================================
  Files           106      106              
  Lines          7042     7047       +5     
  Branches        368      370       +2     
============================================
  Hits           5727     5727              
- Misses         1117     1121       +4     
- Partials        198      199       +1     
Impacted Files Coverage Δ Complexity Δ
...ble/data/v2/stub/EnhancedBigtableStubSettings.java 96.07% <42.85%> (-1.53%) 23.00 <1.00> (+1.00) ⬇️
...om/google/cloud/bigtable/emulator/v2/Emulator.java 59.83% <0.00%> (-0.82%) 14.00% <0.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 0ef7c5d...1abfa37. Read the comment docs.

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

@kolea2 kolea2 merged commit b33b0fc into googleapis:master Nov 10, 2020
@release-please release-please bot mentioned this pull request Nov 10, 2020
mutianf pushed a commit to mutianf/java-bigtable that referenced this pull request Nov 12, 2020
ad548 pushed a commit to ad548/java-bigtable that referenced this pull request Mar 13, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
api: bigtable Issues related to the googleapis/java-bigtable API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants