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

impl: check if options are empty #9617

Merged
merged 2 commits into from
Aug 2, 2022

Conversation

dbolduc
Copy link
Member

@dbolduc dbolduc commented Aug 2, 2022

Part of the work for #7688

Add an internal helper to see if Options are empty or not.

There are two code paths in the bigtable::Table client. One that uses client_ and one that uses connection_. Per operation options only apply to the path that uses the connection_. I do not want to check if opts.has<T> for each T option. So, we just check to see if the options are empty.

The code in bigtable will look vaguely like this:

Status Table::Foo(Options opts) {
  if (!connection_ && !google::cloud::internal::IsEmpty(opts)) {
    return Status(StatusCode::kFailedPrecondition,
                   "Per-operation options only apply to `Table`s constructed "
                   "with a `DataConnection`.");
  }
  // etc...
}

This change is Reviewable

@dbolduc dbolduc requested a review from a team as a code owner August 2, 2022 15:14
@dbolduc dbolduc marked this pull request as draft August 2, 2022 15:14
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: d1e22d3c499626f863299794a1a7274a85dca516

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #9617 (f49f082) into main (7735b70) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #9617      +/-   ##
==========================================
- Coverage   94.36%   94.36%   -0.01%     
==========================================
  Files        1491     1491              
  Lines      137590   137599       +9     
==========================================
+ Hits       129842   129848       +6     
- Misses       7748     7751       +3     
Impacted Files Coverage Δ
google/cloud/options.h 97.82% <ø> (ø)
google/cloud/options.cc 100.00% <100.00%> (ø)
google/cloud/options_test.cc 100.00% <100.00%> (ø)
...bigtable/examples/bigtable_hello_instance_admin.cc 81.00% <0.00%> (-2.00%) ⬇️
...cloud/pubsub/internal/subscription_session_test.cc 97.98% <0.00%> (-0.34%) ⬇️
google/cloud/pubsub/samples/samples.cc 90.77% <0.00%> (+0.07%) ⬆️

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 7735b70...f49f082. Read the comment docs.

@dbolduc dbolduc marked this pull request as ready for review August 2, 2022 16:35
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: f49f08298e4885d3b659f871ec19f4a4f7c112ea

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@dbolduc dbolduc enabled auto-merge (squash) August 2, 2022 19:37
@dbolduc dbolduc merged commit 919c183 into googleapis:main Aug 2, 2022
@dbolduc dbolduc deleted the bigtable-operation-options-pr-1 branch December 5, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants