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

feat: add toString to Bigtable*Settings to display defaults #235

Closed
wants to merge 1 commit into from

Conversation

rahulKQL
Copy link
Contributor

Fixes #233

This PR adds toString() to all three BigtableDataSettings,BigtableTableAdminSettings, BigtableInstanceAdminSettings. This is to help user log the current settings.

This would display the default in the following manner:

BigtableDataSettings{projectId=fake-project, instanceId=fake-instance, appProfileId=fake-app-profile, stubSettings=EnhancedBigtableStubSettings{executorProvider=InstantiatingExecutorProvider{executorThreadCount=8, threadFactory=com.google.api.gax.core.InstantiatingExecutorProvider$1@533dcfeb}, transportChannelProvider=com.google.api.gax.grpc.InstantiatingGrpcChannelProvider@8062dcc, credentialsProvider=GoogleCredentialsProvider{scopesToApply=[https://www.googleapis.com/auth/bigtable.data, https://www.googleapis.com/auth/bigtable.data.readonly, https://www.googleapis.com/auth/cloud-bigtable.data, https://www.googleapis.com/auth/cloud-bigtable.data.readonly, https://www.googleapis.com/auth/cloud-platform, https://www.googleapis.com/auth/cloud-platform.read-only], jwtEnabledScopes=[https://www.googleapis.com/auth/bigtable.data, https://www.googleapis.com/auth/cloud-bigtable.data, https://www.googleapis.com/auth/cloud-platform], OAuth2Credentials=null}, headerProvider=com.google.api.gax.rpc.NoHeaderProvider@bab4db70, internalHeaderProvider=com.google.api.gax.rpc.NoHeaderProvider@6d584b5d, clock=com.google.api.core.NanoClock@f33616bc, endpoint=bigtable.googleapis.com:443, streamWatchdogProvider=com.google.api.gax.rpc.InstantiatingWatchdogProvider@885d9266, streamWatchdogCheckInterval=PT10S, tracerFactory=com.google.api.gax.tracing.OpencensusTracerFactory@121cd451}}

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 31, 2020
@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #235 into master will decrease coverage by 0.53%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #235      +/-   ##
============================================
- Coverage     80.69%   80.15%   -0.54%     
  Complexity     1049     1049              
============================================
  Files            99       99              
  Lines          6541     6556      +15     
  Branches        344      322      -22     
============================================
- Hits           5278     5255      -23     
- Misses         1082     1097      +15     
- Partials        181      204      +23     
Impacted Files Coverage Δ Complexity Δ
...gtable/admin/v2/BigtableInstanceAdminSettings.java 84.84% <0.00%> (-11.71%) 6.00 <0.00> (ø)
.../bigtable/admin/v2/BigtableTableAdminSettings.java 58.33% <0.00%> (-5.31%) 8.00 <0.00> (ø)
...e/cloud/bigtable/data/v2/BigtableDataSettings.java 61.76% <0.00%> (-5.98%) 7.00 <0.00> (ø)
...igtable/admin/v2/BaseBigtableTableAdminClient.java 58.18% <0.00%> (-4.73%) 67.00% <0.00%> (ø%)
...able/admin/v2/BaseBigtableInstanceAdminClient.java 57.07% <0.00%> (-4.40%) 56.00% <0.00%> (ø%)
...om/google/cloud/bigtable/emulator/v2/Emulator.java 57.39% <0.00%> (-0.87%) 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 ac0997d...f7e1b79. Read the comment docs.

@igorbernstein2
Copy link
Contributor

I think we can improve on this:

The main thing that this is missing is the per-rpc settings: what's the total timeout for readRows? what's exponential backoff is configured for mutateRow?

@igorbernstein2 igorbernstein2 added the needs work This is a pull request that needs a little love. label Mar 31, 2020
@igorbernstein2
Copy link
Contributor

@rahulKQL is this unblocked now?

@rahulKQL
Copy link
Contributor Author

Unfortunately, this still blocked due to #1017 from gax is yet to release.

Copy link
Collaborator

@kolea2 kolea2 left a comment

Choose a reason for hiding this comment

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

to be reviewed

This PR adds toString() to all three BigtableDataSettings,BigtableTableAdminSettings, BigtableInstanceAdminSettings. This is to help user log the current settings.
public String toString() {
return MoreObjects.toStringHelper(this)
.add("projectId", projectId)
.add("stubSettings", stubSettings)
Copy link
Collaborator

@kolea2 kolea2 Jul 15, 2020

Choose a reason for hiding this comment

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

does this need instanceId, or is it part of this?

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable API. label Oct 7, 2020
@igorbernstein2
Copy link
Contributor

Closing in favor #488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/java-bigtable API. cla: yes This human has signed the Contributor License Agreement. needs work This is a pull request that needs a little love.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement toString for Bigtable*Settings
5 participants