Skip to content

feat(bigtable): Add support for adding tags to an Instance in InstanceConfig #16014

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

Merged
scotthart merged 4 commits into from
Jun 24, 2026
+35 −0
Merged

feat(bigtable): Add support for adding tags to an Instance in InstanceConfig #16014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afb33db
Add support for adding tags to an Instance in InstanceConfig
dohun-dev Mar 2, 2026
73e5367
add AI review suggestion of support for rvalue in emplace_tag
dohun-dev Mar 3, 2026
386232a
Merge branch 'main' into tags
scotthart Jun 24, 2026
5a441a5
Fix lint errors
dohun-dev Jun 24, 2026
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
16 changes: 16 additions & 0 deletions google/cloud/bigtable/instance_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,22 @@ class InstanceConfig {
return *this;
}

InstanceConfig& insert_tag(std::string const& key, std::string const& value) {
(*proto_.mutable_instance()->mutable_tags())[key] = value;
return *this;
}

InstanceConfig& emplace_tag(std::string const& key, std::string value) {
(*proto_.mutable_instance()->mutable_tags())[key] = std::move(value);
return *this;
}
Comment on lines +75 to +78

@gemini-code-assist gemini-code-assist Bot Mar 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To optimize for cases where the key is an rvalue (e.g., a temporary from a string literal), consider adding an overload for emplace_tag that accepts an rvalue reference for the key. This allows moving the key into the map instead of copying it, improving efficiency.

A similar improvement could be made to emplace_label in a follow-up change for consistency.

  InstanceConfig& emplace_tag(std::string const& key, std::string value) {
    (*proto_.mutable_instance()->mutable_tags())[key] = std::move(value);
    return *this;
  }

  InstanceConfig& emplace_tag(std::string&& key, std::string value) {
    (*proto_.mutable_instance()->mutable_tags())[std::move(key)] =
        std::move(value);
    return *this;
  }

@dohun-dev dohun-dev Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


InstanceConfig& emplace_tag(std::string&& key, std::string value) {

@mutianf mutianf Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do we need this overload? I know this is probably added because of the code assist, but looking at insert_label and emplace_label, it only has std::string const& key version. maybe we should just mirror that first.

@scotthart scotthart Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's fine to leave it. The *_label functions could be similarly improved.

(*proto_.mutable_instance()->mutable_tags())[std::move(key)] =
std::move(value);
return *this;
}

// NOLINT: accessors can (and should) be snake_case.
google::bigtable::admin::v2::CreateInstanceRequest const& as_proto() const& {
return proto_;
Expand Down
19 changes: 19 additions & 0 deletions google/cloud/bigtable/instance_config_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,25 @@ TEST(InstanceConfigTest, SetLabels) {
EXPECT_EQ("qux", proto.instance().labels().at("baz"));
}

TEST(InstanceConfigTest, SetTags) {
InstanceConfig config("my-instance", "pretty name",
{
{"cluster-1", {"somewhere", 7, ClusterConfig::SSD}},
});

config.insert_tag("tagKeys/123", "tagValues/654")
.emplace_tag("tagKeys/345", "tagValues/987");

auto proto = config.as_proto();
EXPECT_EQ("my-instance", proto.instance_id());
EXPECT_EQ("pretty name", proto.instance().display_name());
ASSERT_EQ(1, proto.clusters_size());

ASSERT_EQ(2, proto.instance().tags_size());
EXPECT_EQ("tagValues/654", proto.instance().tags().at("tagKeys/123"));
EXPECT_EQ("tagValues/987", proto.instance().tags().at("tagKeys/345"));
}

} // namespace
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace bigtable
Expand Down
Loading