cleanup(bigtable): use Status factory functions (#14223)

* cleanup(bigtable): use `Status` factory functions * checkers and fix * fix * fix clang and generate libraries
googleapis · May 13, 2024 · 0b1549b · 0b1549b
1 parent e469a54
commit 0b1549b
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 71 deletions.
diff --git a/google/cloud/bigtable/benchmarks/benchmark.cc b/google/cloud/bigtable/benchmarks/benchmark.cc
@@ -18,6 +18,7 @@
 #include "google/cloud/bigtable/resource_names.h"
 #include "google/cloud/internal/background_threads_impl.h"
 #include "google/cloud/internal/getenv.h"
+#include "google/cloud/internal/make_status.h"
 #include <future>
 #include <iomanip>
 #include <sstream>
@@ -44,8 +45,8 @@ google::cloud::StatusOr<BenchmarkOptions> ParseArgs(
       if (!value.empty()) continue;
       std::ostringstream os;
       os << "The environment variable " << var << " is not set or empty";
-      return google::cloud::Status(google::cloud::StatusCode::kUnknown,
-                                   std::move(os).str());
+      return google::cloud::internal::UnknownError(std::move(os).str(),
+                                                   GCP_ERROR_INFO());
     }
     auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value();
     auto const instance_id =

diff --git a/google/cloud/bigtable/benchmarks/mutation_batcher_throughput_benchmark.cc b/google/cloud/bigtable/benchmarks/mutation_batcher_throughput_benchmark.cc
@@ -20,6 +20,7 @@
 #include "google/cloud/bigtable/testing/random_names.h"
 #include "google/cloud/internal/background_threads_impl.h"
 #include "google/cloud/internal/getenv.h"
+#include "google/cloud/internal/make_status.h"
 #include "google/cloud/status_or.h"
 #include "google/cloud/testing_util/command_line_parsing.h"
 #include "absl/time/time.h"
@@ -36,7 +37,6 @@ namespace cbt = ::google::cloud::bigtable;
 using cbt::benchmarks::MutationBatcherThroughputOptions;
 using cbt::benchmarks::ParseMutationBatcherThroughputOptions;
 using ::google::cloud::Status;
-using ::google::cloud::StatusCode;
 using ::google::cloud::StatusOr;
 using ::google::cloud::internal::GetEnv;
 
@@ -93,7 +93,8 @@ StatusOr<MutationBatcherThroughputOptions> ParseArgs(int argc, char* argv[]) {
       if (!value.empty()) continue;
       std::ostringstream os;
       os << "The environment variable " << var << "is not set or empty";
-      return Status(StatusCode::kUnknown, std::move(os).str());
+      return google::cloud::internal::UnknownError(std::move(os).str(),
+                                                   GCP_ERROR_INFO());
     }
     auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value();
     auto const instance_id =

diff --git a/google/cloud/bigtable/benchmarks/mutation_batcher_throughput_options.cc b/google/cloud/bigtable/benchmarks/mutation_batcher_throughput_options.cc
@@ -14,6 +14,7 @@
 
 #include "google/cloud/bigtable/benchmarks/mutation_batcher_throughput_options.h"
 #include "google/cloud/internal/absl_str_join_quiet.h"
+#include "google/cloud/internal/make_status.h"
 #include "google/cloud/status_or.h"
 #include "google/cloud/testing_util/command_line_parsing.h"
 #include <sstream>
@@ -103,8 +104,7 @@ ParseMutationBatcherThroughputOptions(std::vector<std::string> const& argv,
   }
 
   auto make_status = [](std::ostringstream& os) {
-    auto const code = google::cloud::StatusCode::kInvalidArgument;
-    return google::cloud::Status{code, std::move(os).str()};
+    return internal::InvalidArgumentError(std::move(os).str());
   };
 
   if (unparsed.size() != 1) {

diff --git a/google/cloud/bigtable/client_options.h b/google/cloud/bigtable/client_options.h
@@ -22,6 +22,7 @@
 #include "google/cloud/completion_queue.h"
 #include "google/cloud/grpc_options.h"
 #include "google/cloud/internal/algorithm.h"
+#include "google/cloud/internal/make_status.h"
 #include "google/cloud/options.h"
 #include "google/cloud/status.h"
 #include "google/cloud/tracing_options.h"
@@ -365,9 +366,10 @@ class ClientOptions {
         std::chrono::duration_cast<std::chrono::milliseconds>(fallback_timeout);
 
     if (ft_ms.count() > std::numeric_limits<int>::max()) {
-      return google::cloud::Status(google::cloud::StatusCode::kOutOfRange,
-                                   "The supplied duration is larger than the "
-                                   "maximum value allowed by gRPC (INT_MAX)");
+      return google::cloud::internal::OutOfRangeError(
+          "The supplied duration is larger than the "
+          "maximum value allowed by gRPC (INT_MAX)",
+          GCP_ERROR_INFO());
     }
     auto fallback_timeout_ms = static_cast<int>(ft_ms.count());
     opts_.lookup<GrpcChannelArgumentsNativeOption>().SetGrpclbFallbackTimeout(

diff --git a/google/cloud/bigtable/instance_resource.cc b/google/cloud/bigtable/instance_resource.cc
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/bigtable/instance_resource.h"
+#include "google/cloud/internal/make_status.h"
 #include <ostream>
 #include <regex>
 
@@ -44,8 +45,9 @@ StatusOr<InstanceResource> MakeInstanceResource(std::string const& full_name) {
   std::regex re("projects/([^/]+)/instances/([^/]+)");
   std::smatch matches;
   if (!std::regex_match(full_name, matches, re)) {
-    return Status(StatusCode::kInvalidArgument,
-                  "Improperly formatted InstanceResource: " + full_name);
+    return internal::InvalidArgumentError(
+        "Improperly formatted InstanceResource: " + full_name,
+        GCP_ERROR_INFO());
   }
   return InstanceResource(Project(std::move(matches[1])),
                           std::move(matches[2]));

diff --git a/google/cloud/bigtable/internal/bulk_mutator.cc b/google/cloud/bigtable/internal/bulk_mutator.cc
@@ -15,6 +15,7 @@
 #include "google/cloud/bigtable/internal/bulk_mutator.h"
 #include "google/cloud/bigtable/rpc_retry_policy.h"
 #include "google/cloud/bigtable/table.h"
+#include "google/cloud/internal/make_status.h"
 #include "google/cloud/log.h"
 #include <numeric>
 
@@ -146,12 +147,12 @@ void BulkMutatorState::OnFinish(Status finish_status,
       pending_annotations_.push_back(std::move(annotation));
     } else {
       if (last_status_.ok()) {
-        Status status(
-            StatusCode::kInternal,
+        auto status = internal::InternalError(
             "The server never sent a confirmation for this mutation but the "
             "stream didn't fail either. This is most likely a bug, please "
             "report it at "
-            "https://github.com/googleapis/google-cloud-cpp/issues/new");
+            "https://github.com/googleapis/google-cloud-cpp/issues/new",
+            GCP_ERROR_INFO());
         failures_.emplace_back(std::move(status), annotation.original_index);
       } else {
         failures_.emplace_back(last_status_, annotation.original_index);
@@ -173,12 +174,12 @@ std::vector<bigtable::FailedMutation> BulkMutatorState::OnRetryDone() && {
     } else if (!last_status_.ok()) {
       result.emplace_back(last_status_, annotation.original_index);
     } else {
-      Status status(
-          StatusCode::kInternal,
+      auto status = internal::InternalError(
           "The server never sent a confirmation for this mutation but the "
           "stream didn't fail either. This is most likely a bug, please "
           "report it at "
-          "https://github.com/googleapis/google-cloud-cpp/issues/new");
+          "https://github.com/googleapis/google-cloud-cpp/issues/new",
+          GCP_ERROR_INFO());
       result.emplace_back(std::move(status), annotation.original_index);
     }
   }

diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc
@@ -25,6 +25,7 @@
 #include "google/cloud/grpc_options.h"
 #include "google/cloud/idempotency.h"
 #include "google/cloud/internal/async_retry_loop.h"
+#include "google/cloud/internal/make_status.h"
 #include "google/cloud/internal/retry_loop.h"
 #include <memory>
 #include <string>
@@ -227,9 +228,10 @@ StatusOr<std::pair<bool, bigtable::Row>> DataConnectionImpl::ReadRow(
   if (!*it) return it->status();
   auto result = std::make_pair(true, std::move(**it));
   if (++it != reader.end()) {
-    return Status(
-        StatusCode::kInternal,
-        "internal error - RowReader returned more than one row in ReadRow()");
+    return internal::InternalError(
+        "internal error - RowReader returned more than one row in ReadRow(, "
+        ")",
+        GCP_ERROR_INFO());
   }
   return result;
 }

diff --git a/google/cloud/bigtable/table.cc b/google/cloud/bigtable/table.cc
@@ -20,6 +20,7 @@
 #include "google/cloud/bigtable/internal/legacy_row_reader.h"
 #include "google/cloud/bigtable/internal/unary_client_utils.h"
 #include "google/cloud/internal/async_retry_unary_rpc.h"
+#include "google/cloud/internal/make_status.h"
 #include <thread>
 #include <type_traits>
 
@@ -54,9 +55,10 @@ Status Table::Apply(SingleRowMutation mut, Options opts) {
     return connection_->Apply(table_name_, std::move(mut));
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
-    return Status(StatusCode::kInvalidArgument,
-                  "Per-operation options only apply to `Table`s constructed "
-                  "with a `DataConnection`.");
+    return google::cloud::internal::InvalidArgumentError(
+        "Per-operation options only apply to `Table`s constructed "
+        "with a `DataConnection`.",
+        GCP_ERROR_INFO());
   }
 
   // Copy the policies in effect for this operation.  Many policy classes change
@@ -106,10 +108,10 @@ future<Status> Table::AsyncApply(SingleRowMutation mut, Options opts) {
     return connection_->AsyncApply(table_name_, std::move(mut));
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
-    return make_ready_future(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."));
+    return make_ready_future(google::cloud::internal::InvalidArgumentError(
+        "Per-operation options only apply to `Table`s constructed "
+        "with a `DataConnection`.",
+        GCP_ERROR_INFO()));
   }
 
   google::bigtable::v2::MutateRowRequest request;
@@ -156,9 +158,10 @@ std::vector<FailedMutation> Table::BulkApply(BulkMutation mut, Options opts) {
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
     return bigtable_internal::MakeFailedMutations(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."),
+        google::cloud::internal::InvalidArgumentError(
+            "Per-operation options only apply to `Table`s constructed "
+            "with a `DataConnection`.",
+            GCP_ERROR_INFO()),
         mut.size());
   }
 
@@ -196,9 +199,10 @@ future<std::vector<FailedMutation>> Table::AsyncBulkApply(BulkMutation mut,
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
     return make_ready_future(bigtable_internal::MakeFailedMutations(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."),
+        google::cloud::internal::InvalidArgumentError(
+            "Per-operation options only apply to `Table`s constructed "
+            "with a `DataConnection`.",
+            GCP_ERROR_INFO()),
         mut.size()));
   }
 
@@ -225,9 +229,10 @@ RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit,
   if (!google::cloud::internal::IsEmpty(opts)) {
     return MakeRowReader(
         std::make_shared<bigtable_internal::StatusOnlyRowReader>(
-            Status(StatusCode::kInvalidArgument,
-                   "Per-operation options only apply to `Table`s constructed "
-                   "with a `DataConnection`.")));
+            google::cloud::internal::InvalidArgumentError(
+                "Per-operation options only apply to `Table`s constructed "
+                "with a `DataConnection`.",
+                GCP_ERROR_INFO())));
   }
 
   auto impl = std::make_shared<bigtable_internal::LegacyRowReader>(
@@ -246,9 +251,10 @@ StatusOr<std::pair<bool, Row>> Table::ReadRow(std::string row_key,
                                 std::move(filter));
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
-    return Status(StatusCode::kInvalidArgument,
-                  "Per-operation options only apply to `Table`s constructed "
-                  "with a `DataConnection`.");
+    return google::cloud::internal::InvalidArgumentError(
+        "Per-operation options only apply to `Table`s constructed "
+        "with a `DataConnection`.",
+        GCP_ERROR_INFO());
   }
 
   RowSet row_set(std::move(row_key));
@@ -265,9 +271,9 @@ StatusOr<std::pair<bool, Row>> Table::ReadRow(std::string row_key,
   }
   auto result = std::make_pair(true, std::move(**it));
   if (++it != reader.end()) {
-    return Status(
-        StatusCode::kInternal,
-        "internal error - RowReader returned more than one row in ReadRow()");
+    return google::cloud::internal::InternalError(
+        "internal error - RowReader returned more than one row in ReadRow(, "
+        "GCP_ERROR_INFO())");
   }
   return result;
 }
@@ -282,9 +288,10 @@ StatusOr<MutationBranch> Table::CheckAndMutateRow(
         std::move(true_mutations), std::move(false_mutations));
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
-    return Status(StatusCode::kInvalidArgument,
-                  "Per-operation options only apply to `Table`s constructed "
-                  "with a `DataConnection`.");
+    return google::cloud::internal::InvalidArgumentError(
+        "Per-operation options only apply to `Table`s constructed "
+        "with a `DataConnection`.",
+        GCP_ERROR_INFO());
   }
 
   grpc::Status status;
@@ -325,9 +332,10 @@ future<StatusOr<MutationBranch>> Table::AsyncCheckAndMutateRow(
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
     return make_ready_future<StatusOr<MutationBranch>>(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."));
+        google::cloud::internal::InvalidArgumentError(
+            "Per-operation options only apply to `Table`s constructed "
+            "with a `DataConnection`.",
+            GCP_ERROR_INFO()));
   }
 
   btproto::CheckAndMutateRowRequest request;
@@ -382,9 +390,10 @@ StatusOr<std::vector<bigtable::RowKeySample>> Table::SampleRows(Options opts) {
     return connection_->SampleRows(table_name_);
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
-    return Status(StatusCode::kInvalidArgument,
-                  "Per-operation options only apply to `Table`s constructed "
-                  "with a `DataConnection`.");
+    return google::cloud::internal::InvalidArgumentError(
+        "Per-operation options only apply to `Table`s constructed "
+        "with a `DataConnection`.",
+        GCP_ERROR_INFO());
   }
 
   // Copy the policies in effect for this operation.
@@ -435,9 +444,10 @@ future<StatusOr<std::vector<bigtable::RowKeySample>>> Table::AsyncSampleRows(
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
     return make_ready_future<StatusOr<std::vector<RowKeySample>>>(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."));
+        google::cloud::internal::InvalidArgumentError(
+            "Per-operation options only apply to `Table`s constructed "
+            "with a `DataConnection`.",
+            GCP_ERROR_INFO()));
   }
 
   auto cq = background_threads_->cq();
@@ -456,9 +466,10 @@ StatusOr<Row> Table::ReadModifyWriteRowImpl(
     return connection_->ReadModifyWriteRow(std::move(request));
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
-    return Status(StatusCode::kInvalidArgument,
-                  "Per-operation options only apply to `Table`s constructed "
-                  "with a `DataConnection`.");
+    return google::cloud::internal::InvalidArgumentError(
+        "Per-operation options only apply to `Table`s constructed "
+        "with a `DataConnection`.",
+        GCP_ERROR_INFO());
   }
 
   grpc::Status status;
@@ -484,9 +495,10 @@ future<StatusOr<Row>> Table::AsyncReadModifyWriteRowImpl(
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
     return make_ready_future<StatusOr<Row>>(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."));
+        google::cloud::internal::InvalidArgumentError(
+            "Per-operation options only apply to `Table`s constructed "
+            "with a `DataConnection`.",
+            GCP_ERROR_INFO()));
   }
 
   auto cq = background_threads_->cq();
@@ -522,9 +534,10 @@ future<StatusOr<std::pair<bool, Row>>> Table::AsyncReadRow(std::string row_key,
   }
   if (!google::cloud::internal::IsEmpty(opts)) {
     return make_ready_future<StatusOr<std::pair<bool, Row>>>(
-        Status(StatusCode::kInvalidArgument,
-               "Per-operation options only apply to `Table`s constructed "
-               "with a `DataConnection`."));
+        google::cloud::internal::InvalidArgumentError(
+            "Per-operation options only apply to `Table`s constructed "
+            "with a `DataConnection`.",
+            GCP_ERROR_INFO()));
   }
 
   class AsyncReadRowHandler {

diff --git a/google/cloud/bigtable/table.h b/google/cloud/bigtable/table.h
@@ -37,6 +37,7 @@
 #include "google/cloud/future.h"
 #include "google/cloud/grpc_error_delegate.h"
 #include "google/cloud/internal/group_options.h"
+#include "google/cloud/internal/make_status.h"
 #include "google/cloud/options.h"
 #include "google/cloud/status.h"
 #include "google/cloud/status_or.h"
@@ -783,10 +784,10 @@ class Table {
       return;
     }
     if (!google::cloud::internal::IsEmpty(opts)) {
-      on_finish_fn(
-          Status(StatusCode::kInvalidArgument,
-                 "Per-operation options only apply to `Table`s constructed "
-                 "with a `DataConnection`."));
+      on_finish_fn(google::cloud::internal::InvalidArgumentError(
+          "Per-operation options only apply to `Table`s constructed "
+          "with a `DataConnection`.",
+          GCP_ERROR_INFO()));
       return;
     }