feat(spanner): spanner::Client construction from Options #7706
Conversation
Add a new `spanner::Client` constructor using `google::cloud::Options`. This can then be used to merge with per-operation `Options` in a follow up PR. `ClientOptions` construction is still supported for compatibility, but it is immediately converted to an `Options` value. `QueryOptions` is still a first-class citizen, however, because of its use in the `Connection` interface.
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #7706 +/- ##
=======================================
Coverage 95.29% 95.30%
=======================================
Files 1254 1256 +2
Lines 113620 113697 +77
=======================================
+ Hits 108273 108354 +81
+ Misses 5347 5343 -4
Continue to review full report at Codecov.
|
| * | ||
| * Note that the previous constructor, which uses the new way to represent | ||
| * options of all varieties (`google::cloud::Options`), is now preferred. | ||
| */ |
There was a problem hiding this comment.
I planned to leave deprecating ClientOptions to a later stage.
| * @note This constructor exists solely for backwards compatibility. It | ||
| * prevents existing code that uses `Client(conn, {})` from breaking | ||
| * due to ambiguity introduced by the `Options` overload. | ||
| */ |
There was a problem hiding this comment.
In pubsub, I introduced this constructor as @deprecated, to scare away any users reading the documentation. In the long run, we do want to remove the function, but, we also want Client(conn, {}) to be valid code that calls the Options constructor. (So I'm not sure using @deprecated was correct).
Changing from a @note to a @warning is also possible. I'll let you decide if you think either of these are improvements.
There was a problem hiding this comment.
Yeah, I don't think @deprecated is best for the reasons you cite. And indeed, @warning falls to the same logic methinks. The @note is really just an answer to "What's this?", and it all goes away when ClientOptions does.
| * Convert the `ClientOptions` to the new, recommended way to represent | ||
| * options of all varieties, `google::cloud::Options`. | ||
| */ | ||
| explicit operator Options() const; |
There was a problem hiding this comment.
in pubsub a conversion method like this was hidden away in internal with the logic being that ClientOptions was to be deprecated, so no public methods should be added to it.
There was a problem hiding this comment.
I was thinking that a customer might want to call this as part of their own migration, but I'm just guessing. In any case, with ClientOptions on the road to removal I'm not too concerned about adding to the API.
Add a new
spanner::Clientconstructor usinggoogle::cloud::Options.This can then be used to merge with per-operation
Optionsin a followup PR.
ClientOptionsconstruction is still supported for compatibility, butit is immediately converted to an
Optionsvalue.QueryOptionsis still a first-class citizen, however, because of its use in the
Connectioninterface.This change is