Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: uses old version of gax-grpc method #426

Conversation

thiagotnunes
Copy link
Contributor

@thiagotnunes thiagotnunes commented Sep 15, 2020

Internally, we are using version 1.47.1 of the gax-grpc library, which does not provide a method currently being used (setExecutor).

In here we use a method that is available in 1.47.1 and achieves the same goal (setExecutorProvider).

@google-cla google-cla bot added the cla: yes label Sep 15, 2020
@codecov
Copy link

@codecov codecov bot commented Sep 15, 2020

Codecov Report

Merging #426 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #426      +/-   ##
============================================
- Coverage     82.16%   82.14%   -0.03%     
- Complexity     2455     2459       +4     
============================================
  Files           136      136              
  Lines         13589    13589              
  Branches       1307     1307              
============================================
- Hits          11166    11163       -3     
- Misses         1895     1897       +2     
- Partials        528      529       +1     
Impacted Files Coverage Δ Complexity Δ
...m/google/cloud/spanner/spi/v1/GapicSpannerRpc.java 82.03% <100.00%> (ø) 81.00 <0.00> (ø)
...va/com/google/cloud/spanner/AbstractResultSet.java 81.53% <0.00%> (-0.92%) 28.00% <0.00%> (ø%)
.../google/cloud/spanner/SpannerExceptionFactory.java 74.35% <0.00%> (+2.56%) 34.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 b312091...4ed38fe. Read the comment docs.

@thiagotnunes thiagotnunes requested review from skuruppu and olavloite Sep 15, 2020
// Before updating this method to setExecutor, please make sure to verify the version
// of gax-grpc used internally. At the moment we are using 1.47.1, which does not
// support the setExecutor variant.
Copy link
Contributor

@skuruppu skuruppu Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment may not give enough context for folks outside of Google. So instead reword to something like:

"Before updating this method to setExecutor, please verify with a code owner on the lowest version of gax-grpc that needs to be supported. Currently v1.47.17, which doesn't support the setExecutor variant."

Copy link
Contributor Author

@thiagotnunes thiagotnunes Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Internally, we are using version 1.47.1 of the gax-grpc library, which
does not provide a method currently being used (setExecutor).

In here we use a method that is available in 1.47.1 and achieves the
same goal (setExecutorProvider).
@thiagotnunes thiagotnunes force-pushed the use-old-version-of-gax-grpc-method branch from 94fd5bb to 4ed38fe Compare Sep 15, 2020
@thiagotnunes thiagotnunes merged commit fe6dc79 into googleapis:master Sep 15, 2020
17 of 18 checks passed
@thiagotnunes thiagotnunes deleted the use-old-version-of-gax-grpc-method branch Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants