apply review comments

Author: sophia-bq

aws_advanced_python_wrapper/aurora_initial_connection_strategy_plugin.py

@@ -213,10 +213,11 @@ def _get_reader(self, props: Properties) -> Optional[HostInfo]:
                 if url_type and url_type.has_region:
                     aws_region = self._rds_utils.get_rds_region(original_host.host)
                     if aws_region:
-                        hosts_in_region = [
-                            h for h in self._plugin_service.all_hosts
-                            if aws_region.lower() == self._rds_utils.get_rds_region(h.host).lower()
-                        ]
+                        hosts_in_region = []
+                        for h in self._plugin_service.all_hosts:
+                            h_region = self._rds_utils.get_rds_region(h.host)
+                            if h_region and aws_region.lower() == h_region.lower():
+                                hosts_in_region.append(h)
                         return self._plugin_service.get_host_info_by_strategy(
                             HostRole.READER, strategy, hosts_in_region)
 

aws_advanced_python_wrapper/database_dialect.py

@@ -64,7 +64,7 @@ class DialectCode(Enum):
     MYSQL = "mysql"
 
     MULTI_AZ_CLUSTER_PG = "multi-az-pg"
-    GLOBAL_AURORA_PG = "aurora-pg"
+    GLOBAL_AURORA_PG = "global-aurora-pg"
     AURORA_PG = "aurora-pg"
     RDS_PG = "rds-pg"
     PG = "pg"
@@ -547,7 +547,7 @@ class GlobalAuroraMysqlDialect(AuroraMysqlDialect, GlobalAuroraTopologyDialect):
          "FROM information_schema.aurora_global_db_instance_status ")
     _REGION_COUNT_QUERY = "SELECT count(1) FROM information_schema.aurora_global_db_status"
     _REGION_BY_INSTANCE_ID_QUERY = \
-        "SELECT AWS_REGION FROM information_schema.aurora_global_db_instance_status WHERE SERVER_ID = ?"
+        "SELECT AWS_REGION FROM information_schema.aurora_global_db_instance_status WHERE SERVER_ID = %s"
 
     @property
     def dialect_update_candidates(self) -> Optional[Tuple[DialectCode, ...]]:
@@ -564,10 +564,10 @@ def is_dialect(self, conn: Connection, driver_dialect: DriverDialect) -> bool:
             with closing(conn.cursor()) as cursor:
                 cursor.execute(self._REGION_COUNT_QUERY)
                 record = cursor.fetchone()
-                if record is None or len(record) < 2:
+                if record is None or len(record) < 1:
                     return False
 
-                aws_region_count = record[1]
+                aws_region_count = record[0]
                 return aws_region_count is not None and aws_region_count > 1
         except Exception:
             if not initial_transaction_status and driver_dialect.is_in_transaction(conn):
@@ -592,7 +592,7 @@ class GlobalAuroraPgDialect(AuroraPgDialect, GlobalAuroraTopologyDialect):
          "FROM aurora_global_db_instance_status()")
     _REGION_COUNT_QUERY = "SELECT count(1) FROM aurora_global_db_status()"
     _REGION_BY_INSTANCE_ID_QUERY = \
-        "SELECT AWS_REGION FROM aurora_global_db_instance_status() WHERE SERVER_ID = ?"
+        "SELECT AWS_REGION FROM aurora_global_db_instance_status() WHERE SERVER_ID = %s"
 
     @property
     def dialect_update_candidates(self) -> Optional[Tuple[DialectCode, ...]]:
@@ -620,10 +620,10 @@ def is_dialect(self, conn: Connection, driver_dialect: DriverDialect) -> bool:
             with closing(conn.cursor()) as cursor:
                 cursor.execute(self._REGION_COUNT_QUERY)
                 record = cursor.fetchone()
-                if record is None or len(record) < 2:
+                if record is None or len(record) < 1:
                     return False
 
-                aws_region_count = record[1]
+                aws_region_count = record[0]
                 return aws_region_count is not None and aws_region_count > 1
 
         except Exception:

aws_advanced_python_wrapper/host_list_provider.py

@@ -848,14 +848,14 @@ def _process_global_query_results(
             else:
                 hosts.append(host)
 
-        if len(writers) == 0:
+        if not writers:
             logger.error("RdsHostListProvider.InvalidTopology")
             hosts.clear()
         elif len(writers) == 1:
             hosts.append(writers[0])
         else:
             existing_writers: List[HostInfo] = [x for x in writers if x is not None]
-            existing_writers.sort(reverse=True, key=lambda h: h.last_update_time is not None and h.last_update_time)
+            existing_writers.sort(reverse=True, key=lambda h: h.last_update_time or datetime.min)
             hosts.append(existing_writers[0])
 
         return tuple(hosts)

aws_advanced_python_wrapper/utils/region_utils.py

@@ -49,7 +49,7 @@ def get_region_from_hostname(self, hostname: Optional[str]) -> Optional[str]:
 
 
 class GdbRegionUtils(RegionUtils):
-    _GDB_CLUSTER_ARN_PATTERN = r"^arn:aws:rds:(?P<region>[^:\n]*):[^:\n]*:([^:/\n]*[:/])?(.*)$"
+    _GDB_CLUSTER_ARN_PATTERN = r"^arn:aws[^:]*:rds:(?P<region>[^:\n]*):[^:\n]*:([^:/\n]*[:/])?(.*)$"
     _REGION_GROUP = "region"
 
     def get_region(self,

tests/unit/test_dialect.py

@@ -427,7 +427,7 @@ def test_query_for_dialect_mysql(mock_conn, mock_cursor, mock_driver_dialect):
 def test_global_aurora_is_dialect_with_global_tables(mock_conn, mock_cursor, mock_driver_dialect):
     mock_conn.cursor.return_value = mock_cursor
     mock_cursor.__enter__.return_value = mock_cursor
-    mock_cursor.fetchone.side_effect = [(1,), (1,), (None, 2)]
+    mock_cursor.fetchone.side_effect = [(1,), (1,), (2,)]
 
     dialect = GlobalAuroraMysqlDialect()
     assert dialect.is_dialect(mock_conn, mock_driver_dialect) is True