Skip to content

Conversation

@dbolduc
Copy link
Member

@dbolduc dbolduc commented Jan 21, 2022

Part of the work for #7534

We need to store the project id ourselves, so we can eventually drop our dependence on the legacy client_.

The full change (short of a CHANGELOG entry) is here: https://github.com/dbolduc/google-cloud-cpp/compare/bigtable-convert-instance-admin

The diff is 4500 lines. I am trying to break it up into something more palatable. My plan is to break it into the following PRs:

  1. project id change (this PR)
  2. add the logic for storing a generated Connection (without ever using the new connection)
  3. add the mechanism to convert bigtable policies passed to the constructor into Options.
  4. (BREAKING): Reimplement InstanceAdmin in terms of the generated Connection. Rewrite the unit tests. Update the CHANGELOG. (This will be a tedious PR).
  5. Gut the InstanceAdminClient class. Delete unused classes and their tests.

This change is Reviewable

@dbolduc dbolduc requested a review from a team as a code owner January 21, 2022 19:36
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Jan 21, 2022
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: f6481e49c3f4898108a8d6ed4c373c640581513c

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

@dbolduc dbolduc marked this pull request as draft January 21, 2022 19:42
@dbolduc dbolduc force-pushed the bigtable-legacy-instance-admin-project-id branch from f6481e4 to 43a035f Compare January 21, 2022 19:46
@dbolduc dbolduc marked this pull request as ready for review January 21, 2022 19:47
@dbolduc dbolduc enabled auto-merge (squash) January 21, 2022 19:47
@google-cloud-cpp-bot
Copy link
Collaborator

Google Cloud Build Logs
For commit: 43a035f4cdb98ff899b1c2c040d5cd26c2fac236

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

@codecov
Copy link

codecov bot commented Jan 21, 2022

Codecov Report

Merging #8051 (43a035f) into main (ebc8f09) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8051      +/-   ##
==========================================
- Coverage   95.11%   95.10%   -0.01%     
==========================================
  Files        1313     1313              
  Lines      117659   117662       +3     
==========================================
- Hits       111907   111905       -2     
- Misses       5752     5757       +5     
Impacted Files Coverage Δ
google/cloud/bigtable/instance_admin.h 100.00% <100.00%> (ø)
...e/cloud/spanner/testing/cleanup_stale_instances.cc 61.29% <0.00%> (-19.36%) ⬇️
...ud/spanner/integration_tests/client_stress_test.cc 85.52% <0.00%> (-0.66%) ⬇️
.../cloud/storage/benchmarks/throughput_experiment.cc 74.37% <0.00%> (-0.51%) ⬇️
...cloud/pubsub/internal/subscription_session_test.cc 98.00% <0.00%> (+0.24%) ⬆️
...le/cloud/internal/default_completion_queue_impl.cc 97.72% <0.00%> (+0.56%) ⬆️
google/cloud/bigtable/internal/common_client.cc 97.14% <0.00%> (+1.42%) ⬆️

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 ebc8f09...43a035f. Read the comment docs.

@dbolduc dbolduc merged commit 0d77e69 into googleapis:main Jan 21, 2022
@dbolduc dbolduc deleted the bigtable-legacy-instance-admin-project-id branch January 21, 2022 21:21
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 Bigtable API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants