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.

Sign up for GitHub

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

Jump to bottom

add spanner support for per-operation options #7690

Closed
Tracked by #8164
devbww opened this issue Dec 2, 2021 · 1 comment
Closed
Tracked by #8164

add spanner support for per-operation options #7690

devbww opened this issue Dec 2, 2021 · 1 comment
Assignees
@devbww
Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@devbww
Copy link
Contributor

devbww commented Dec 2, 2021

Add an Options options = {} argument to the constructor of the client class(es). Merge these options with the default options for the service, and store them as a member of the client class.

Add an Options options = {} argument to each operation within each client. These options should then be merged with the client options from above, and installed as the prevailing options for the duration of the operation by instantiating an internal::OptionsSpan.

You could then use internal::CurrentOptions() to obtain (a const& to) the prevailing options from anywhere you might need them. Any cleanup for call paths where Options have been passed explicitly is discretionary.

Similar support for the generated client classes was added in #7683, so you might be able to use that as an example.
@devbww devbww added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. api: spanner Issues related to the Spanner API. labels Dec 2, 2021
@devbww devbww self-assigned this Dec 2, 2021
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Dec 9, 2021
@devbww
feat(spanner): add per-operation options to Commit() and Rollback()
e3fd368 
Part of googleapis#7690 to add an `Options opts = {}` argument to `Commit()`
and `Rollback()` operations.  Add new `TransactionTagOption` and
`CommitReturnStatsOption` types to accompany `RequestPriorityOption`.
This means the caller no longer needs `CommitOptions` (although it is
still used in the `Connection` layer).

Run the `Client` options through `spanner_internal::DefaultOptions()`.
Note that per-operation options strictly override those client-level
options.

Add `OptionsSpan` local variables to `Commit()` and `Rollback()`
(and also `ExecuteBatchDml()`, which already took plain `Options`).

Clarify which constructor and operation overloads are for backwards
compatibility so that they will be easier to deprecate/remove later.
Also clarify some doxygen comments about `Options` parameters.

Finally, update the `Client` test and sample code to use the new
options.
@devbww devbww mentioned this issue Dec 9, 2021
Merged
devbww added a commit that referenced this issue Dec 9, 2021
@devbww
feat(spanner): add per-operation options to Commit() and Rollback() (#…
fc44681 
…7714)

Part of #7690 to add an `Options opts = {}` argument to `Commit()`
and `Rollback()` operations.  Add new `TransactionTagOption` and
`CommitReturnStatsOption` types to accompany `RequestPriorityOption`.
This means the caller no longer needs `CommitOptions` (although it is
still used in the `Connection` layer).

Run the `Client` options through `spanner_internal::DefaultOptions()`.
Note that per-operation options strictly override those client-level
options.

Add `OptionsSpan` local variables to `Commit()` and `Rollback()`
(and also `ExecuteBatchDml()`, which already took plain `Options`).

Clarify which constructor and operation overloads are for backwards
compatibility so that they will be easier to deprecate/remove later.
Also clarify some doxygen comments about `Options` parameters.

Finally, update the `Client` test and sample code to use the new
options.
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Jan 25, 2022
@devbww
feat(spanner): merge connection options into client options
882fa02 
Store the `Connection` options so that they may be made available to
merge into the `Client` options.

Use those stored options to implement four `Connection` data elements
that were previously extracted during construction.  (When all `Client`
operations have been taught how to set the "current options" we will
be able to use those instead.)

Part of googleapis#7690.
@devbww devbww mentioned this issue Jan 25, 2022
Merged
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Jan 25, 2022
@devbww
feat(spanner): merge connection options into client options
d819840 
Store the `Connection` options so that they may be made available to
merge into the `Client` options.

Use those stored options to implement four `Connection` data elements
that were previously extracted during construction.  (When all `Client`
operations have been taught how to set the "current options" we will
be able to use those instead.)

Part of googleapis#7690.
devbww added a commit that referenced this issue Jan 26, 2022
@devbww
feat(spanner): merge connection options into client options (#8090)
2e33f18 
Store the `Connection` options so that they may be made available to
merge into the `Client` options.

Use those stored options to implement four `Connection` data elements
that were previously extracted during construction.  (When all `Client`
operations have been taught how to set the "current options" we will
be able to use those instead.)

Part of #7690.
@devbww
Copy link
Contributor Author

devbww commented Feb 18, 2022

#8355 was also part of this, to handle streaming RPCs.

@devbww devbww mentioned this issue Feb 22, 2022
Closed
8 tasks
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Feb 22, 2022
@devbww
feat(spanner): convert QueryOptions to Options in main client API
bbea0bf 
Add an `Options opts = {}` argument to `ExecuteQuery()`, `ProfileQuery()`,
`ExecuteDml()`, `ProfileDml()`, `AnalyzeSql()`, and `ExecutePartitionedDml()`,
and relegate their `QueryOptions` overloads to the "backwards compatibility"
section.  The new `Options` overloads also begin an `OptionsSpan`.

Add `operator QueryOptions::Options()` to facilitate those compatibility
interfaces, and add a round-trip test against `QueryOptions(Options const&)`.
Then use that to implement `operator ClientOptions::Options()` to eliminate
some duplication (and `client_options.cc`).

Move the `SPANNER_OPTIMIZER_VERSION`/`SPANNER_OPTIMIZER_STATISTICS_PACKAGE`
handling into `spanner:: DefaultOptions()`, which means that all of the
`OverlayQueryOptions()` machinery can go away.  "Overlay" priorities are
now handled by the normal `Options`-merging process.

Update the samples to use the preferred `Options` overloads.

Part of googleapis#7690.

Aside: The existing doxygen groupings do not appear to do what we want
here, but let's continue with them for now, and fix them en masse later.
@devbww devbww mentioned this issue Feb 22, 2022
Merged
devbww added a commit that referenced this issue Feb 23, 2022
@devbww
feat(spanner): convert QueryOptions to Options in main client API (#8430
7913d6c 
)

Add an `Options opts = {}` argument to `ExecuteQuery()`, `ProfileQuery()`,
`ExecuteDml()`, `ProfileDml()`, `AnalyzeSql()`, and `ExecutePartitionedDml()`,
and relegate their `QueryOptions` overloads to the "backwards compatibility"
section.  The new `Options` overloads also begin an `OptionsSpan`.

Add `operator QueryOptions::Options()` to facilitate those compatibility
interfaces, and add a round-trip test against `QueryOptions(Options const&)`.
Then use that to implement `operator ClientOptions::Options()` to eliminate
some duplication (and `client_options.cc`).

Move the `SPANNER_OPTIMIZER_VERSION`/`SPANNER_OPTIMIZER_STATISTICS_PACKAGE`
handling into `spanner:: DefaultOptions()`, which means that all of the
`OverlayQueryOptions()` machinery can go away.  "Overlay" priorities are
now handled by the normal `Options`-merging process.

Update the samples to use the preferred `Options` overloads.

Part of #7690.
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Feb 24, 2022
@devbww
feat(spanner): convert ReadOptions/PartitionOptions to Options
bed081d 
Add an `Options opts = {}` argument to `Read()`, `PartitionRead()`, and
`PartitionQuery()`, and relegate their `ReadOptions` and `PartitionOptions`
overloads to the "backwards compatibility" section.  The new `Options`
overloads also begin an `OptionsSpan`.

Add `ReadOptions`/`PartitionOptions`-to/from-`Options` conversion functions
to facilitate those compatibility interfaces, and add round-trip tests.

This required adding new Spanner-specific options `ReadIndexNameOption`,
`ReadRowLimitOption`, `PartitionSizeOption`, and `PartitionsMaximumOption`.

Update the samples to use the preferred `Options` overloads.

Part of googleapis#7690.
@devbww devbww mentioned this issue Feb 24, 2022
Merged
devbww added a commit that referenced this issue Feb 25, 2022
@devbww
feat(spanner): convert ReadOptions/PartitionOptions to Options (#8448)
2ba1571 
Add an `Options opts = {}` argument to `Read()`, `PartitionRead()`, and
`PartitionQuery()`, and relegate their `ReadOptions` and `PartitionOptions`
overloads to the "backwards compatibility" section.  The new `Options`
overloads also begin an `OptionsSpan`.

Add `ReadOptions`/`PartitionOptions`-to/from-`Options` conversion functions
to facilitate those compatibility interfaces, and add round-trip tests.

This required adding new Spanner-specific options `ReadIndexNameOption`,
`ReadRowLimitOption`, `PartitionSizeOption`, and `PartitionsMaximumOption`.

Update the samples to use the preferred `Options` overloads.

Part of #7690.
devbww added a commit to devbww/google-cloud-cpp that referenced this issue Feb 28, 2022
@devbww
feat(spanner): use prevailing Options in ConnectionImpl
2289253 
Allow the prevailing `Options` to override the connection options
for the retry/backoff policies and RPC tracing parameters, so that
any per-operation options are heeded.

This is the final step in fixing googleapis#7690, so announce that users
should now prefer to use `Options` on all Spanner client calls.
@devbww devbww mentioned this issue Feb 28, 2022
Merged
devbww added a commit that referenced this issue Feb 28, 2022
@devbww
feat(spanner): use prevailing Options in ConnectionImpl (#8466)
697cc0a 
Allow the prevailing `Options` to override the connection options
for the retry/backoff policies and RPC tracing parameters, so that
any per-operation options are heeded.

This is the final step in fixing #7690, so announce that users
should now prefer to use `Options` on all Spanner client calls.
@devbww devbww closed this as completed Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@devbww devbww

Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@devbww