feat(bigtable): configure *Clients with EndpointOption
#9082
Conversation
product-auto-label
bot
added
the
api: bigtable
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
dbolduc
force-pushed
the
bigtable-uses-endpoint-option
branch
from
c4953b3 to
d973532
Compare
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #9082 +/- ##
=======================================
Coverage 94.43% 94.43%
=======================================
Files 1430 1430
Lines 127051 127084 +33
=======================================
+ Hits 119978 120015 +37
+ Misses 7073 7069 -4
Continue to review full report at Codecov.
|
| @@ -79,6 +79,13 @@ int DefaultConnectionPoolSize() { | |||
| Options DefaultOptions(Options opts) { | |||
| using ::google::cloud::internal::GetEnv; | |||
|
|
|||
| if (opts.has<EndpointOption>()) { | |||
| auto const& ep = opts.get<EndpointOption>(); | |||
| opts.set<DataEndpointOption>(ep); | |||
There was a problem hiding this comment.
What if the application already provided DataEndpointOption for some reason?
There was a problem hiding this comment.
Seems to me this is a breaking change. The behavior is now different.
There was a problem hiding this comment.
The behavior is only different if you were setting both EndpointOption and *EndpointOption (to different values). That's possible, but it would be really strange. The only case I can think of is reusing one set of options for the Data API and for the generated Admin APIs.
I'll change this PR so that *EndpointOption takes precedence over EndpointOption. It feels weird because I want to do away with the *EndpointOptions. But that's ok.
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Fixes #9027 and does a tiny bit of the work for #9081
This change:
EndpointOptionwithDataClient,AdminClient, andInstanceAdminClientDataEndpointOption,AdminEndpointOption, andInstanceAdminEndpointOptionto configure the above classes in documentation only.Note that our
Default*Options()functions expectDefaultOptions()to provide input via*EndpointOption. (for example here). I will clean this stuff up as part of #9081.This change is