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

feat(bigtable): cheap Table creation with different resource name #8172

Merged
merged 4 commits into from
Jan 31, 2022
Merged

feat(bigtable): cheap Table creation with different resource name #8172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29be6a8
feat(bigtable): edit Table project and instance ids
dbolduc Jan 31, 2022
1d15556
address review comments
dbolduc Jan 31, 2022
e3a9692
address app profile id
dbolduc Jan 31, 2022
033da4a
less bad code
dbolduc Jan 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions google/cloud/bigtable/metadata_update_policy_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,25 @@ TEST_F(MetadataUpdatePolicyTest, RunWithEmbeddedServerParamTableName) {
EXPECT_EQ(expected, range.first->second);
}

/// @test A test for setting metadata when table is known.
TEST_F(MetadataUpdatePolicyTest, ModifiedTableName) {
std::string const new_project_id = "modified-project";
std::string const new_instance_id = "modified-instance";
table_->set_project_id(new_project_id);
table_->set_instance_id(new_instance_id);

grpc::string expected =
"table_name=" + TableName(new_project_id, new_instance_id, kTableId);
auto reader = table_->ReadRows(RowSet("row1"), 1, Filter::PassAllFilter());
// lets make the RPC call to send metadata
reader.begin();
// Get metadata from embedded server
auto client_metadata = bigtable_service_.client_metadata();
auto range = client_metadata.equal_range("x-goog-request-params");
ASSERT_EQ(1, std::distance(range.first, range.second));
EXPECT_EQ(expected, range.first->second);
}

/// @test A cloning test for normal construction of metadata .
TEST_F(MetadataUpdatePolicyTest, SimpleDefault) {
auto const x_google_request_params = "parent=" + std::string(kInstanceName);
Expand Down
27 changes: 24 additions & 3 deletions google/cloud/bigtable/table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "google/cloud/bigtable/idempotent_mutation_policy.h"
#include "google/cloud/bigtable/mutations.h"
#include "google/cloud/bigtable/read_modify_write_rule.h"
#include "google/cloud/bigtable/resource_names.h"
#include "google/cloud/bigtable/row_key_sample.h"
#include "google/cloud/bigtable/row_reader.h"
#include "google/cloud/bigtable/row_set.h"
Expand Down Expand Up @@ -212,7 +213,9 @@ class Table {
std::string const& table_id)
: client_(std::move(client)),
app_profile_id_(std::move(app_profile_id)),
table_name_(TableName(client_, table_id)),
project_id_(client_->project_id()),
instance_id_(client_->instance_id()),
table_name_(TableName(project_id_, instance_id_, table_id)),
table_id_(table_id),
rpc_retry_policy_prototype_(
bigtable::DefaultRPCRetryPolicy(internal::kBigtableLimits)),
Expand Down Expand Up @@ -351,10 +354,26 @@ class Table {

std::string const& table_name() const { return table_name_; }
std::string const& app_profile_id() const { return app_profile_id_; }
std::string const& project_id() const { return client_->project_id(); }
std::string const& instance_id() const { return client_->instance_id(); }
std::string const& project_id() const { return project_id_; }
std::string const& instance_id() const { return instance_id_; }
std::string const& table_id() const { return table_id_; }

/// Override the project id
void set_project_id(std::string project_id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative would be:

Table WithNewTarget(std::string instance_id, std::string project_id, std::string table_id) const {
  auto table = *this;
  table.instance_id_ = std::move(instance_id);
  table.project_id_ = std::move(project_id);
  table.table_id_ = std::move(table_id);
  return table;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Thanks for the idea.

project_id_ = std::move(project_id);
table_name_ = TableName(project_id_, instance_id_, table_id_);
metadata_update_policy_ =
MetadataUpdatePolicy(table_name_, MetadataParamTypes::TABLE_NAME);
}

/// Override the instance id
void set_instance_id(std::string instance_id) {
instance_id_ = std::move(instance_id);
table_name_ = TableName(project_id_, instance_id_, table_id_);
metadata_update_policy_ =
MetadataUpdatePolicy(table_name_, MetadataParamTypes::TABLE_NAME);
}

/**
* Attempts to apply the mutation to a row.
*
Expand Down Expand Up @@ -936,6 +955,8 @@ class Table {
friend class MutationBatcher;
std::shared_ptr<DataClient> client_;
std::string app_profile_id_;
std::string project_id_;
std::string instance_id_;
std::string table_name_;
std::string table_id_;
std::shared_ptr<RPCRetryPolicy const> rpc_retry_policy_prototype_;
Expand Down
17 changes: 17 additions & 0 deletions google/cloud/bigtable/table_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ TEST_F(TableTest, StandaloneTableName) {

TEST_F(TableTest, TableName) { EXPECT_EQ(kTableName, table_.table_name()); }

TEST_F(TableTest, OverrideFields) {
Table table(client_, kTableId);
EXPECT_EQ(table.project_id(), kProjectId);
EXPECT_EQ(table.instance_id(), kInstanceId);
EXPECT_EQ(table.table_name(), TableName(kProjectId, kInstanceId, kTableId));

std::string const new_project_id = "modified-project";
std::string const new_instance_id = "modified-instance";
table.set_project_id(new_project_id);
table.set_instance_id(new_instance_id);

EXPECT_EQ(table.project_id(), new_project_id);
EXPECT_EQ(table.instance_id(), new_instance_id);
EXPECT_EQ(table.table_name(),
TableName(new_project_id, new_instance_id, kTableId));
}

TEST_F(TableTest, TableConstructor) {
std::string const other_table_id = "my-table";
std::string const other_table_name = TableName(client_, other_table_id);
Expand Down