Address PR issues

Author: mrambacher

cloud/aws/aws_env.cc

@@ -76,7 +76,7 @@ Status AwsCloudAccessCredentials::TEST_Initialize() {
 Status AwsCloudAccessCredentials::CheckCredentials(
     const AwsAccessType& aws_type) const {
 #ifndef USE_AWS
-  (void) aws_type;
+  (void)aws_type;
   return Status::NotSupported("AWS not supported");
 #else
   if (aws_type == AwsAccessType::kSimple) {

cloud/aws/aws_kafka.cc

@@ -203,8 +203,6 @@ class KafkaController : public CloudLogControllerImpl {
   virtual CloudLogWritableFile* CreateWritableFile(const std::string& fname,
                                                    const EnvOptions& options) override;
 
-  Status Verify() const override;
-
  protected:
   Status Initialize(CloudEnv* env) override;
 
@@ -240,12 +238,11 @@ Status KafkaController::Initialize(CloudEnv* env) {
   for (auto const& item : kconf) {
     if (conf->set(item.first, item.second, conf_errstr) !=
         RdKafka::Conf::CONF_OK) {
-      s = Status::InvalidArgument(
-          "Failed adding specified conf to Kafka conf", conf_errstr.c_str());
+      s = Status::InvalidArgument("Failed adding specified conf to Kafka conf",
+                                  conf_errstr.c_str());
 
       Log(InfoLogLevel::ERROR_LEVEL, env->info_log_,
-          "[aws] NewAwsEnv Kafka conf set error: %s",
-          s.ToString().c_str());
+          "[aws] NewAwsEnv Kafka conf set error: %s", s.ToString().c_str());
       return s;
     }
   }
@@ -288,24 +285,6 @@ Status KafkaController::Initialize(CloudEnv* env) {
   return s;
 }
 
-Status KafkaController::Verify() const {
-  Status s = CloudLogControllerImpl::Verify();
-  if (s.ok()) {
-    if (!producer_) {
-      s = Status::InvalidArgument("Failed creating Kafka producer");
-    } else if (!consumer_) {
-      s = Status::InvalidArgument("Failed creating Kafka consumer");
-    } else if (producer_topic_ == nullptr) {
-      s = Status::InvalidArgument("Failed to initialize Kafa Producer Topic");
-    } else if (consumer_topic_ == nullptr) {
-      s = Status::InvalidArgument("Failed to initialize Kafa Consumer Topic");
-    } else if (consuming_queue_ == nullptr) {
-      s = Status::InvalidArgument("Failed to initialize Kafa Consuming Queue");
-    }
-  }
-  return s;
-}
-
 Status KafkaController::TailStream() {
   InitializePartitions();
 

cloud/aws/aws_kinesis.cc

@@ -189,8 +189,6 @@ class KinesisController : public CloudLogControllerImpl {
 
   CloudLogWritableFile* CreateWritableFile(const std::string& fname,
                                            const EnvOptions& options) override;
-  Status Verify() const override;
-
  protected:
   Status Initialize(CloudEnv* env) override;
 
@@ -212,40 +210,30 @@ class KinesisController : public CloudLogControllerImpl {
 };
 
 Status KinesisController::Initialize(CloudEnv* env) {
-  Status status = CloudLogControllerImpl::Initialize(env);
-  if (status.ok()) {
+  Status st = CloudLogControllerImpl::Initialize(env);
+  if (st.ok()) {
     Aws::Client::ClientConfiguration config;
     const auto& options = env->GetCloudEnvOptions();
     std::shared_ptr<Aws::Auth::AWSCredentialsProvider> provider;
-    status = options.credentials.GetCredentialsProvider(&provider);
-    if (status.ok()) {
-      status = AwsCloudOptions::GetClientConfiguration(
+    st = options.credentials.GetCredentialsProvider(&provider);
+    if (st.ok()) {
+      st = AwsCloudOptions::GetClientConfiguration(
           env, options.src_bucket.GetRegion(), &config);
     }
-    if (status.ok()) {
+    if (st.ok()) {
       kinesis_client_.reset(
           provider ? new Aws::Kinesis::KinesisClient(provider, config)
                    : new Aws::Kinesis::KinesisClient(config));
       // Initialize stream name.
       std::string bucket = env->GetSrcBucketName();
       topic_ = Aws::String(bucket.c_str(), bucket.size());
 
-      Log(InfoLogLevel::DEBUG_LEVEL, env->info_log_,
+      Log(InfoLogLevel::INFO_LEVEL, env->info_log_,
           "[%s] KinesisController opening stream %s using cachedir '%s'",
           Name(), topic_.c_str(), cache_dir_.c_str());
     }
   }
-  return status;
-}
-
-Status KinesisController::Verify() const {
-  Status s = CloudLogControllerImpl::Verify();
-  if (s.ok()) {
-    if (!kinesis_client_) {
-      s = Status::InvalidArgument("Failed to initialize kinesis client");
-    }
-  }
-  return s;
+  return st;
 }
 
 Status KinesisController::TailStream() {

cloud/aws/aws_s3.cc

@@ -33,7 +33,7 @@
 #include <aws/s3/model/PutObjectResult.h>
 #include <aws/s3/model/ServerSideEncryption.h>
 #include <aws/transfer/TransferManager.h>
-#endif // USE_AWS
+#endif  // USE_AWS
 
 #include <cassert>
 #include <cinttypes>
@@ -389,8 +389,6 @@ class S3StorageProvider : public CloudStorageProviderImpl {
                               const std::string& object_path,
                               std::unique_ptr<CloudStorageWritableFile>* result,
                               const EnvOptions& options) override;
-  Status Verify() const override;
-
  protected:
   Status Initialize(CloudEnv* env) override;
   Status DoGetObject(const std::string& bucket_name,
@@ -451,16 +449,6 @@ Status S3StorageProvider::Initialize(CloudEnv* env) {
   return status;
 }
 
-Status S3StorageProvider::Verify() const {
-  Status s = CloudStorageProviderImpl::Verify();
-  if (s.ok()) {
-    if (!s3client_) {
-      s = Status::InvalidArgument("S3Client Failed to initialize");
-    }
-  }
-  return s;
-}
-
 //
 // Create bucket in S3 if it does not already exist.
 //

cloud/cloud_env_impl.cc

@@ -999,24 +999,6 @@ Status CloudEnvImpl::RollNewEpoch(const std::string& local_dbname) {
   return Status::OK();
 }
 
-Status CloudEnvImpl::Verify() const {
-  Status s;
-  if (!cloud_env_options.storage_provider) {
-    s = Status::InvalidArgument(
-        "Cloud environment requires a storage provider");
-  } else {
-    s = cloud_env_options.storage_provider->Verify();
-  }
-  if (s.ok()) {
-    if (cloud_env_options.cloud_log_controller) {
-      s = cloud_env_options.cloud_log_controller->Verify();
-    } else if (!cloud_env_options.keep_local_log_files) {
-      s = Status::InvalidArgument(
-          "Log controller required for remote log files");
-    }
-  }
-  return s;
-}
 Status CloudEnvImpl::Prepare() {
   Header(info_log_, "     %s.src_bucket_name: %s", Name(),
          cloud_env_options.src_bucket.GetBucketName().c_str());

cloud/cloud_env_impl.h

@@ -106,7 +106,6 @@ class CloudEnvImpl : public CloudEnv {
 
  protected:
   virtual Status Prepare();
-  virtual Status Verify() const;
 
   // Does the dir need to be re-initialized?
   Status NeedsReinitialization(const std::string& clone_dir, bool* do_reinit);

cloud/cloud_log_controller.cc

@@ -76,15 +76,6 @@ Status CloudLogControllerImpl::Prepare(CloudEnv* env) {
   return status_;
 }
 
-Status CloudLogControllerImpl::Verify() const {
-  if (!status_.ok()) {
-    return status_;
-  } else if (!env_) {
-    return Status::InvalidArgument("Log Controller not initialized: ", Name());
-  }
-  return status_;
-}
-
 std::string CloudLogControllerImpl::GetCachePath(
     const Slice& original_pathname) const {
   const std::string & cache_dir = GetCacheDir();

cloud/cloud_log_controller_impl.h

@@ -55,8 +55,6 @@ class CloudLogControllerImpl : public CloudLogController {
   Status FileExists(const std::string& fname) override;
   Status GetFileSize(const std::string& logical_fname, uint64_t* size) override;
   virtual Status Prepare(CloudEnv* env) override;
-  virtual Status Verify() const override;
-
  protected:
   virtual Status Initialize(CloudEnv* env);
   // Converts an original pathname to a pathname in the cache.

cloud/cloud_storage_provider.cc

@@ -286,17 +286,6 @@ CloudStorageProviderImpl::CloudStorageProviderImpl() : rng_(time(nullptr)) {}
 
 CloudStorageProviderImpl::~CloudStorageProviderImpl() {}
 
-Status CloudStorageProviderImpl::Verify() const {
-  if (!status_.ok()) {
-    return status_;
-  } else if (!env_) {
-    return Status::InvalidArgument("Storage Provider not initialized: ",
-                                   Name());
-  } else {
-    return Status::OK();
-  }
-}
-
 Status CloudStorageProviderImpl::Prepare(CloudEnv* env) {
   status_ = Initialize(env);
   if (status_.ok()) {

cloud/cloud_storage_provider_impl.h

@@ -117,8 +117,6 @@ class CloudStorageProviderImpl : public CloudStorageProvider {
                               std::unique_ptr<CloudStorageReadableFile>* result,
                               const EnvOptions& options) override;
   virtual Status Prepare(CloudEnv* env) override;
-  virtual Status Verify() const override;
-
  protected:
   Random64 rng_;
   virtual Status Initialize(CloudEnv* env);