chore: address review comments

Author: karenc-bq

driver/custom_endpoint_monitor.cc

@@ -93,11 +93,11 @@ bool CUSTOM_ENDPOINT_MONITOR::has_custom_endpoint_info() const {
 }
 
 void CUSTOM_ENDPOINT_MONITOR::run() {
-  MYLOG_TRACE(this->logger, 0, "Starting custom endpoint monitor for '%s'", this->custom_endpoint_host.c_str());
   if (thread_pool.size() == 1) {
     // Each monitor should only have 1 thread.
     return;
   }
+  MYLOG_TRACE(this->logger, 0, "Starting custom endpoint monitor for '%s'", this->custom_endpoint_host.c_str());
   thread_pool.resize(1);
   thread_pool.push([=](int id) {
     ++SDK_HELPER;

driver/failover_handler.cc

@@ -511,7 +511,7 @@ bool FAILOVER_HANDLER::failover_to_reader(const char*& new_error_code, const cha
     if (result->connected) {
         current_host = result->new_host;
         connection_handler->update_connection(result->new_connection, current_host->get_host());
-        new_error_code = "08S02";
+        new_error_code = "08S02"; // Failover succeeded error code.
         error_msg = "The active SQL connection has changed.";
         MYLOG_DBC_TRACE(dbc,
                     "[FAILOVER_HANDLER] The active SQL connection has changed "
@@ -520,7 +520,7 @@ bool FAILOVER_HANDLER::failover_to_reader(const char*& new_error_code, const cha
         return true;
     } else {
         MYLOG_DBC_TRACE(dbc, "[FAILOVER_HANDLER] Unable to establish SQL connection to reader node.");
-        new_error_code = "08S01";
+        new_error_code = "08S01"; // Failover failed error code.
         error_msg = "The active SQL connection was lost.";
         return false;
     }
@@ -549,7 +549,7 @@ bool FAILOVER_HANDLER::failover_to_writer(const char*& new_error_code, const cha
     const auto filtered_topology = this->topology_service->get_filtered_topology(new_topology);
     const auto allowed_hosts = filtered_topology->get_instances();
     if (std::find(allowed_hosts.begin(), allowed_hosts.end(), new_host) == allowed_hosts.end()) {
-        new_error_code = "08S01";
+        new_error_code = "08S01"; // Failover failed error code.
         error_msg = "The active SQL connection was lost.";
         MYLOG_DBC_TRACE(
             dbc,
@@ -562,7 +562,7 @@ bool FAILOVER_HANDLER::failover_to_writer(const char*& new_error_code, const cha
 
     connection_handler->update_connection(result->new_connection, new_host->get_host());
 
-    new_error_code = "08S02";
+    new_error_code = "08S02"; // Failover succeeded error code.
     error_msg = "The active SQL connection has changed.";
     MYLOG_DBC_TRACE(
         dbc,

driver/host_info.cc

@@ -56,33 +56,25 @@ HOST_INFO::HOST_INFO(const char* host, int port, HOST_STATE state, bool is_write
 HOST_INFO::~HOST_INFO() {}
 
 /**
- * Returns the host endpoint.
- *
  * @return the host endpoint
  */
 std::string HOST_INFO::get_host() {
     return host;
 }
 
 /**
- * Returns the host name.
- *
  * @return the host name
  */
 std::string HOST_INFO::get_host_id() { return host_id; }
 
 /**
- * Returns the port.
- *
  * @return the port
  */
 int HOST_INFO::get_port() {
     return port;
 }
 
 /**
- * Returns a host:port representation of this host.
- *
  * @return the host:port representation of this host
  */
 std::string HOST_INFO::get_host_port_pair() {

driver/topology_service.cc

@@ -191,21 +191,13 @@ std::shared_ptr<CLUSTER_TOPOLOGY_INFO> TOPOLOGY_SERVICE::get_filtered_topology(s
     std::set<std::string> blocked_list = this->allowed_and_blocked_hosts->get_blocked_host_ids();
 
     const std::shared_ptr<CLUSTER_TOPOLOGY_INFO> filtered_topology = std::make_shared<CLUSTER_TOPOLOGY_INFO>();
-    if (allowed_list.size() > 0) {
-        for (const auto& host : topology->get_instances()) {
-            if (allowed_list.find(host->get_host_id()) != allowed_list.end()) {
-                filtered_topology->add_host(host);
-            }
+    for (const auto& host : topology->get_instances()) {
+        if (allowed_list.find(host->get_host_id()) != allowed_list.end()
+        || blocked_list.find(host->get_host_id()) == blocked_list.end()) {
+            filtered_topology->add_host(host);
         }
     }
 
-    if (blocked_list.size() > 0) {
-      for (const auto& host : topology->get_instances()) {
-            if (blocked_list.find(host->get_host_id()) == blocked_list.end()) {
-                filtered_topology->add_host(host);
-            }
-        }
-    }
     return filtered_topology;
 }
 

integration/connection_string_builder.h

@@ -205,4 +205,4 @@ class ConnectionStringBuilder {
   int length = 0;
 };
 
-#endif /* __CONNECTIONSTRINGBUILDER_H__ */
+#endif /* __CONNECTIONSTRINGBUILDER_H__ */

integration/custom_endpoint_integration_test.cc

@@ -52,9 +52,13 @@ class CustomEndpointIntegrationTest : public BaseFailoverIntegrationTest {
 
   static void TearDownTestSuite() { Aws::ShutdownAPI(options); }
   void SetUp() override {
-    SQLAllocHandle(SQL_HANDLE_ENV, nullptr, &env);
+    if (SQLAllocHandle(SQL_HANDLE_ENV, nullptr, &env) != SQL_SUCCESS) {
+      throw std::runtime_error("Failed to allocate handles for integration tests.");
+    }
     SQLSetEnvAttr(env, SQL_ATTR_ODBC_VERSION, reinterpret_cast<SQLPOINTER>(SQL_OV_ODBC3), 0);
-    SQLAllocHandle(SQL_HANDLE_DBC, env, &dbc);
+    if (SQLAllocHandle(SQL_HANDLE_DBC, env, &dbc) != SQL_SUCCESS) {
+      throw std::runtime_error("Failed to allocate handles for integration tests.");
+    }
 
     Aws::Auth::AWSCredentials credentials =
         SESSION_TOKEN.empty() ? Aws::Auth::AWSCredentials(Aws::String(ACCESS_KEY), Aws::String(SECRET_ACCESS_KEY))