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: local connection checker ignores exceptions #1036

Merged
merged 2 commits into from Apr 6, 2021

Conversation

thiagotnunes
Copy link
Contributor

@thiagotnunes thiagotnunes commented Apr 6, 2021

The local connection checker should only check for unavailable exceptions when warning the user about the spanner emulator configuration. It should ignore all other kinds of errors.

The local connection checker should only check for unavailable
exceptions when warning the user about the spanner emulator
configuration. It should ignore all other kinds of errors.
@thiagotnunes thiagotnunes requested a review from as a code owner Apr 6, 2021
@product-auto-label product-auto-label bot added the api: spanner label Apr 6, 2021
@google-cla google-cla bot added the cla: yes label Apr 6, 2021
@codecov
Copy link

@codecov codecov bot commented Apr 6, 2021

Codecov Report

Merging #1036 (f8874d6) into master (70c5b80) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1036      +/-   ##
============================================
+ Coverage     85.20%   85.21%   +0.01%     
- Complexity     2642     2644       +2     
============================================
  Files           155      155              
  Lines         14413    14412       -1     
  Branches       1348     1348              
============================================
+ Hits          12280    12281       +1     
+ Misses         1564     1562       -2     
  Partials        569      569              
Impacted Files Coverage Δ Complexity Δ
...oud/spanner/connection/LocalConnectionChecker.java 81.57% <100.00%> (-0.48%) 6.00 <0.00> (ø)
...ain/java/com/google/cloud/spanner/SessionPool.java 88.93% <0.00%> (-0.39%) 72.00% <0.00%> (-1.00%)
...ud/spanner/SessionPoolAsyncTransactionManager.java 87.21% <0.00%> (+1.50%) 14.00% <0.00%> (+2.00%)
...m/google/cloud/spanner/connection/SpannerPool.java 89.47% <0.00%> (+1.57%) 33.00% <0.00%> (ø%)
.../google/cloud/spanner/AbstractLazyInitializer.java 100.00% <0.00%> (+7.14%) 5.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 70c5b80...f8874d6. Read the comment docs.

@thiagotnunes thiagotnunes requested review from olavloite and zoercai Apr 6, 2021
@Test
public void testNoRunningEmulator() {
final String uri =
"cloudspanner://localhost:42424/projects/proj/instances/inst/databases/db?usePlainText=true";
Copy link
Contributor

@olavloite olavloite Apr 6, 2021

Choose a reason for hiding this comment

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

nit: It might be safer to use server.getPort() - 1 instead of 42424, as there is a 1/65000 chance that the port assigned to the mock server in this test happens to be 42424.

Copy link
Contributor Author

@thiagotnunes thiagotnunes Apr 6, 2021

Choose a reason for hiding this comment

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

Good point, updated it.

@thiagotnunes thiagotnunes added the kokoro:force-run label Apr 6, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Apr 6, 2021
@thiagotnunes thiagotnunes removed the cla: yes label Apr 6, 2021
@google-cla google-cla bot added the cla: yes label Apr 6, 2021
@thiagotnunes thiagotnunes merged commit 2d61bc4 into master Apr 6, 2021
15 of 16 checks passed
@thiagotnunes thiagotnunes deleted the local-checker-ignores-exceptions branch Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants