Skip to content

Commit bc35c1f

Browse files
coryancoryan
authored
cleanup(bigtable): enable function-cognitive-complexity check (#9440)
There were only 3 functions over the limit, only one seems hard to fix. I simply disabled the check for it.
1 parent 2e1a647 commit bc35c1f

4 files changed

Lines changed: 69 additions & 64 deletions

File tree

‎google/cloud/bigtable/.clang-tidy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
InheritParentConfig: true
3+
4+
Checks: >
5+
readability-function-cognitive-complexity

‎google/cloud/bigtable/benchmarks/mutation_batcher_throughput_benchmark.cc‎

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,47 @@ cbt::SingleRowMutation MakeMutation(
126126
{cbt::SetCell(options.column_family, options.column, "value")});
127127
}
128128

129+
StatusOr<std::string> CreateTableIfNeeded(
130+
google::cloud::bigtable_admin::BigtableTableAdminClient admin,
131+
MutationBatcherThroughputOptions const& options, int key_width) {
132+
using ::google::cloud::bigtable::testing::RandomTableId;
133+
if (options.table_id.empty()) {
134+
auto generator = google::cloud::internal::MakeDefaultPRNG();
135+
auto table_id = RandomTableId(generator);
136+
137+
google::bigtable::admin::v2::CreateTableRequest r;
138+
r.set_parent(cbt::InstanceName(options.project_id, options.instance_id));
139+
r.set_table_id(table_id);
140+
// Provide initial splits to the table
141+
for (auto i = 0; i != options.shard_count; ++i) {
142+
auto row_index = options.mutation_count * i / options.shard_count;
143+
r.add_initial_splits()->set_key(MakeRowString(key_width, row_index));
144+
}
145+
auto& families = *r.mutable_table()->mutable_column_families();
146+
families[options.column_family].mutable_gc_rule()->set_max_num_versions(10);
147+
148+
std::cout << "# Creating Table\n";
149+
auto table = admin.CreateTable(std::move(r));
150+
if (!table) {
151+
std::cout << table.status() << std::endl;
152+
return std::move(table).status();
153+
}
154+
std::cout << "#\n";
155+
return table_id;
156+
}
157+
google::bigtable::admin::v2::GetTableRequest r;
158+
r.set_name(cbt::TableName(options.project_id, options.instance_id,
159+
options.table_id));
160+
r.set_view(google::bigtable::admin::v2::Table_View_NAME_ONLY);
161+
auto table = admin.GetTable(std::move(r));
162+
if (!table) {
163+
std::cout << "Error trying to get Table " << options.table_id << ":\n"
164+
<< table.status() << std::endl;
165+
return std::move(table).status();
166+
}
167+
return options.table_id;
168+
}
169+
129170
} // namespace
130171

131172
int main(int argc, char* argv[]) {
@@ -143,7 +184,6 @@ int main(int argc, char* argv[]) {
143184
using ::google::cloud::Options;
144185
using ::google::cloud::Status;
145186
using ::google::cloud::StatusOr;
146-
using ::google::cloud::bigtable::testing::RandomTableId;
147187
using ::google::cloud::internal::AutomaticallyCreatedBackgroundThreads;
148188
using TimerFuture = future<StatusOr<std::chrono::system_clock::time_point>>;
149189

@@ -154,52 +194,18 @@ int main(int argc, char* argv[]) {
154194
for (auto i = options->mutation_count - 1; i != 0; i /= 10) ++key_width;
155195

156196
// Create a new table if one was not supplied
157-
auto table_id = options->table_id;
158-
if (table_id.empty()) {
159-
auto generator = google::cloud::internal::MakeDefaultPRNG();
160-
table_id = RandomTableId(generator);
161-
162-
google::bigtable::admin::v2::CreateTableRequest r;
163-
r.set_parent(cbt::InstanceName(options->project_id, options->instance_id));
164-
r.set_table_id(table_id);
165-
// Provide initial splits to the table
166-
for (auto i = 0; i != options->shard_count; ++i) {
167-
auto row_index = options->mutation_count * i / options->shard_count;
168-
r.add_initial_splits()->set_key(MakeRowString(key_width, row_index));
169-
}
170-
auto& families = *r.mutable_table()->mutable_column_families();
171-
families[options->column_family].mutable_gc_rule()->set_max_num_versions(
172-
10);
173-
174-
std::cout << "# Creating Table\n";
175-
auto table = admin.CreateTable(std::move(r));
176-
if (!table) {
177-
std::cout << table.status() << std::endl;
178-
return 1;
179-
}
180-
std::cout << "#\n";
181-
} else {
182-
google::bigtable::admin::v2::GetTableRequest r;
183-
r.set_name(
184-
cbt::TableName(options->project_id, options->instance_id, table_id));
185-
r.set_view(google::bigtable::admin::v2::Table_View_NAME_ONLY);
186-
auto table = admin.GetTable(std::move(r));
187-
if (!table) {
188-
std::cout << "Error trying to get Table " << table_id << ":\n"
189-
<< table.status() << std::endl;
190-
return 1;
191-
}
192-
}
197+
auto table_id = CreateTableIfNeeded(admin, *options, key_width);
198+
if (!table_id) return 1;
193199

194200
auto table = cbt::Table(
195201
cbt::MakeDataConnection(
196202
Options{}.set<google::cloud::GrpcBackgroundThreadPoolSizeOption>(
197203
options->max_batches)),
198-
cbt::TableResource(options->project_id, options->instance_id, table_id));
204+
cbt::TableResource(options->project_id, options->instance_id, *table_id));
199205

200206
std::cout << "# Project ID: " << options->project_id
201207
<< "\n# Instance ID: " << options->instance_id
202-
<< "\n# Table ID: " << table_id << "\n# Cutoff Time: "
208+
<< "\n# Table ID: " << *table_id << "\n# Cutoff Time: "
203209
<< absl::FormatDuration(absl::FromChrono(options->max_time))
204210
<< "\n# Shard Count: " << options->shard_count
205211
<< "\n# Write Thread Count: " << options->write_thread_count
@@ -309,7 +315,7 @@ int main(int argc, char* argv[]) {
309315
if (options->table_id.empty()) {
310316
std::cout << "#\n# Deleting Table\n";
311317
auto status = admin.DeleteTable(
312-
cbt::TableName(options->project_id, options->instance_id, table_id));
318+
cbt::TableName(options->project_id, options->instance_id, *table_id));
313319
if (!status.ok()) {
314320
std::cout << status << std::endl;
315321
return -1;

‎google/cloud/bigtable/internal/defaults.cc‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -105,30 +105,23 @@ Options DefaultOptions(Options opts) {
105105
opts.set<InstanceAdminEndpointOption>(*std::move(instance_admin_emulator));
106106
}
107107

108-
if (!opts.has<DataEndpointOption>()) {
109-
opts.set<DataEndpointOption>("bigtable.googleapis.com");
110-
}
111-
if (!opts.has<AdminEndpointOption>()) {
112-
opts.set<AdminEndpointOption>("bigtableadmin.googleapis.com");
113-
}
114-
if (!opts.has<InstanceAdminEndpointOption>()) {
115-
opts.set<InstanceAdminEndpointOption>("bigtableadmin.googleapis.com");
116-
}
117-
if (!opts.has<GrpcCredentialOption>()) {
118-
opts.set<GrpcCredentialOption>(emulator ? grpc::InsecureChannelCredentials()
119-
: grpc::GoogleDefaultCredentials());
120-
}
121-
if (!opts.has<TracingComponentsOption>()) {
122-
opts.set<TracingComponentsOption>(
123-
::google::cloud::internal::DefaultTracingComponents());
124-
}
125-
if (!opts.has<GrpcTracingOptionsOption>()) {
126-
opts.set<GrpcTracingOptionsOption>(
127-
::google::cloud::internal::DefaultTracingOptions());
128-
}
129-
if (!opts.has<GrpcNumChannelsOption>()) {
130-
opts.set<GrpcNumChannelsOption>(DefaultConnectionPoolSize());
131-
}
108+
// Fill any missing default values.
109+
auto defaults =
110+
Options{}
111+
.set<DataEndpointOption>("bigtable.googleapis.com")
112+
.set<AdminEndpointOption>("bigtableadmin.googleapis.com")
113+
.set<InstanceAdminEndpointOption>("bigtableadmin.googleapis.com")
114+
.set<GrpcCredentialOption>(emulator
115+
? grpc::InsecureChannelCredentials()
116+
: grpc::GoogleDefaultCredentials())
117+
.set<TracingComponentsOption>(
118+
::google::cloud::internal::DefaultTracingComponents())
119+
.set<GrpcTracingOptionsOption>(
120+
::google::cloud::internal::DefaultTracingOptions())
121+
.set<GrpcNumChannelsOption>(DefaultConnectionPoolSize());
122+
123+
opts = google::cloud::internal::MergeOptions(std::move(opts),
124+
std::move(defaults));
132125

133126
auto const has_min = opts.has<MinConnectionRefreshOption>();
134127
auto const has_max = opts.has<MaxConnectionRefreshOption>();

‎google/cloud/bigtable/internal/readrowsparser.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
2222
namespace internal {
2323
using ::google::bigtable::v2::ReadRowsResponse_CellChunk;
2424

25+
// NOLINTNEXTLINE(readability-function-cognitive-complexity)
2526
void ReadRowsParser::HandleChunk(ReadRowsResponse_CellChunk chunk,
2627
grpc::Status& status) {
2728
if (end_of_stream_) {

0 commit comments

Comments
 (0)