feat(bigtable): support x-goog-user-project#8324
Conversation
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #8324 +/- ##
==========================================
- Coverage 94.97% 94.96% -0.01%
==========================================
Files 1345 1345
Lines 119817 119871 +54
==========================================
+ Hits 113793 113839 +46
- Misses 6024 6032 +8
Continue to review full report at Codecov.
|
dbolduc
left a comment
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @coryan and @dbolduc)
google/cloud/bigtable/data_client.cc, line 116 at r1 (raw file):
ReadRows(grpc::ClientContext* context, btproto::ReadRowsRequest const& request) override { return impl_.Stub()->ReadRows(context, request);
I think we need one here.
google/cloud/bigtable/internal/defaults_test.cc, line 164 at r1 (raw file):
env = ScopedEnvironment("GOOGLE_CLOUD_CPP_USER_PROJECT", "env-project"); options = Options{}.set<UserProjectOption>("env-project");
s/env-project/test-project/
to make sure the user supplied options gets overridden by the env var
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
|
|
||
| TEST(OptionsTest, InstanceAdminUserProjectOption) { | ||
| auto env = ScopedEnvironment("GOOGLE_CLOUD_CPP_USER_PROJECT", absl::nullopt); | ||
| auto options = DefaultInstanceAdminOptions( |
There was a problem hiding this comment.
I would be fine with there just being one test, which calls DefaultOptions( ... ), instead of a test for each service... but your call.
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
|
Can you please undo the changes to |
coryan
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @coryan and @dbolduc)
google/cloud/bigtable/data_client.cc, line 116 at r1 (raw file):
Previously, dbolduc (Darren Bolduc) wrote…
I think we need one here.
Done.
google/cloud/bigtable/internal/defaults_test.cc, line 164 at r1 (raw file):
Previously, dbolduc (Darren Bolduc) wrote…
s/env-project/test-project/
to make sure the user supplied options gets overridden by the env var
Fixed. Also fixed the test to be useful. And also added tests for admin and instance admin.
But they exist now? People can pull code from |
|
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Ok fine. When the time comes, I will delete this with pleasure. |
Part of the work for #8284
This change is