You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "WJSGDBZ (via GitHub)" <gi...@apache.org> on 2025/10/16 07:14:11 UTC

[PR] feat(Python-client):add dns resolution support [incubator-pegasus]

WJSGDBZ opened a new pull request, #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306

   ### What problem does this PR solve? <!--add issue link with summary if exists-->
   add dns resolution support
   
   ### What is changed and how does it work?
   When the Pegasus operation times out, it will attempt to fetch the topology from meta. If all meta servers fail to respond, it will try to re-resolve the DNS to obtain new IP addresses.
   
   ### Checklist <!--REMOVE the items that are not applicable-->
   
   ##### Tests <!-- At least one of them must be included. -->
   
   - Manual test (add detailed scripts or steps below)
   In local tests, the meta addresses can be successfully re-resolved
   ```
   2025-10-16 14:51:00 [134627073400832] [WARNING] pgclient.py:86 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34803), failure: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.defer.CancelledError'>: 
   ]
   2025-10-16 14:51:00 [134627073400832] [WARNING] pgclient.py:92 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34803), time: 2.0 s, timeout
   2025-10-16 14:51:01 [134627073400832] [INFO] pgclient.py:829 table: CL11020_test_python_metaproxy, checking meta ...
   2025-10-16 14:51:02 [134627073400832] [WARNING] pgclient.py:86 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34803), failure: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.defer.CancelledError'>: 
   ]
   2025-10-16 14:51:02 [134627073400832] [WARNING] pgclient.py:92 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34803), time: 2.0 s, timeout
   2025-10-16 14:51:03 [134627073400832] [INFO] pgclient.py:829 table: CL11020_test_python_metaproxy, checking meta ...
   2025-10-16 14:51:04 [134627073400832] [WARNING] pgclient.py:86 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34802), failure: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.defer.CancelledError'>: 
   ]
   2025-10-16 14:51:04 [134627073400832] [WARNING] pgclient.py:92 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34802), time: 2.0 s, timeout
   2025-10-16 14:51:05 [134627073400832] [INFO] pgclient.py:829 table: CL11020_test_python_metaproxy, checking meta ...
   2025-10-16 14:51:06 [134627073400832] [WARNING] pgclient.py:86 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34802), failure: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.defer.CancelledError'>: 
   ]
   2025-10-16 14:51:06 [134627073400832] [WARNING] pgclient.py:92 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34802), time: 2.0 s, timeout
   2025-10-16 14:51:07 [134627073400832] [INFO] pgclient.py:829 table: CL11020_test_python_metaproxy, checking meta ...
   2025-10-16 14:51:08 [134627073400832] [WARNING] pgclient.py:86 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34802), failure: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.defer.CancelledError'>: 
   ]
   2025-10-16 14:51:08 [134627073400832] [WARNING] pgclient.py:92 peer: IPv4Address(type='TCP', host='10.255.255.254', port=34802), time: 2.0 s, timeout
   2025-10-16 14:51:09 [134627073400832] [INFO] pgclient.py:829 table: CL11020_test_python_metaproxy, checking meta ...
   2025-10-16 14:51:09 [134627073400832] [ERROR] pgclient.py:236 table: CL11020_test_python_metaproxy, connect err: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.ConnectionRefusedError'>: Connection was refused by other side: 111: Connection refused.
   ]
   2025-10-16 14:51:09 [134627073400832] [ERROR] pgclient.py:236 table: CL11020_test_python_metaproxy, connect err: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.ConnectionRefusedError'>: Connection was refused by other side: 111: Connection refused.
   ]
   2025-10-16 14:51:09 [134627073400832] [ERROR] pgclient.py:236 table: CL11020_test_python_metaproxy, connect err: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.ConnectionRefusedError'>: Connection was refused by other side: 111: Connection refused.
   ]
   2025-10-16 14:51:09 [134627073400832] [INFO] pgclient.py:375 resolved hostname xxxxxxx to IP type:host_port_types.kHostTypeIpv4, addr:['xx.xx.xx.xx']
   2025-10-16 14:51:09 [134627073400832] [INFO] pgclient.py:389 removed stale server: 127.0.0.1:34602
   2025-10-16 14:51:09 [134627073400832] [INFO] pgclient.py:389 removed stale server: 127.0.0.1:34603
   2025-10-16 14:51:09 [134627073400832] [INFO] pgclient.py:389 removed stale server: 127.0.0.1:34601
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#discussion_r2803044128


##########
python-client/pypegasus/base/ttypes.py:
##########
@@ -310,8 +310,8 @@ def __ne__(self, other):
 class host_port_types(Enum):
     kHostTypeInvalid = 0
     kHostTypeIpv4 = 1
-    kHostTypeGroup = 2
-
+    kHostTypeIpv6 = 2
+    kHostTypeGroup = 3

Review Comment:
   We'd better keep the value of kHostTypeGroup as it has been set to 2 by the server.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#discussion_r2436532783


##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +248,148 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        self.solved_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False
+            
+    def get_host_type(self, ip_list):
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup
+        elif has_ipv4:
+            return host_port_types.kHostTypeIpv4
+        elif has_ipv6:
+            return host_port_types.kHostTypeIpv6
+        else:
+            return host_port_types.kHostTypeInvalid
+    
+    def resolve_all_ips(self, hostname):
+
+        def extract_ips(result, record_type):
+            answers, _, _ = result
+            ips = []
+            for answer in answers:
+                if record_type == dns.A and isinstance(answer.payload, dns.Record_A):
+                    ips.append(answer.payload.dottedQuad())
+                elif record_type == dns.AAAA and isinstance(answer.payload, dns.Record_AAAA):
+                    if isinstance(answer.payload.address, bytes):
+                        ip6_bytes = answer.payload.address
+                        if len(ip6_bytes) == 16:
+                            ip_str = socket.inet_ntop(socket.AF_INET6, ip6_bytes)
+                            ips.append(ip_str)
+                        else:
+                            print(f"Invalid IPv6 bytes length: {len(ip6_bytes)}")

Review Comment:
   Using print() for error reporting is inconsistent with the rest of the codebase which uses the logger. Replace with logger.warning() or logger.error() to maintain consistent logging practices.



##########
python-client/pypegasus/pgclient.py:
##########
@@ -276,8 +407,16 @@ def got_results(self, res):
                             self.name, result)
                 return result
 
-        logger.error('query partition info err. table: %s err: %s',
-                     self.name, res)
+        # all meta server queries failed, maybe need to re-resolve hostname 
+        # to get new IP addresses
+        self.query_times += 1
+        logger.error('query partition info err(%s) table: %s, query_times: %d, err: %s',
+                    res.err.errno, self.name, self.query_times, res)
+        

Review Comment:
   The code attempts to access res.err.errno, but res is a list of tuples from DeferredList, not an error object with an err attribute. This will cause an AttributeError when the error logging is triggered.
   ```suggestion
           # Collect error info from failed results
           error_details = []
           for (suc, result) in res:
               if not suc:
                   # result is a Failure instance from Twisted
                   err_type = getattr(result.value, '__class__', type(result.value)).__name__ if hasattr(result, 'value') else type(result).__name__
                   err_msg = str(result.value) if hasattr(result, 'value') else str(result)
                   error_details.append(f"{err_type}: {err_msg}")
           logger.error('query partition info failed table: %s, query_times: %d, errors: %s',
                        self.name, self.query_times, "; ".join(error_details))
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -276,8 +407,16 @@ def got_results(self, res):
                             self.name, result)
                 return result
 
-        logger.error('query partition info err. table: %s err: %s',
-                     self.name, res)
+        # all meta server queries failed, maybe need to re-resolve hostname 
+        # to get new IP addresses
+        self.query_times += 1
+        logger.error('query partition info err(%s) table: %s, query_times: %d, err: %s',
+                    res.err.errno, self.name, self.query_times, res)
+        
+        if self.query_times >= MAX_META_QUERY_THRESHOLD and self.solved_times <= MAX_SOLVED_THRESHOLD:

Review Comment:
   The logic uses '<=' for MAX_SOLVED_THRESHOLD but '>=' for MAX_META_QUERY_THRESHOLD, which is inconsistent. Consider using the same comparison operator ('>=' for both) to improve code clarity, or document why different operators are needed.
   ```suggestion
           if self.query_times >= MAX_META_QUERY_THRESHOLD and self.solved_times >= MAX_SOLVED_THRESHOLD:
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +248,148 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        self.solved_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False
+            
+    def get_host_type(self, ip_list):
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup
+        elif has_ipv4:
+            return host_port_types.kHostTypeIpv4
+        elif has_ipv6:
+            return host_port_types.kHostTypeIpv6
+        else:
+            return host_port_types.kHostTypeInvalid
+    
+    def resolve_all_ips(self, hostname):
+
+        def extract_ips(result, record_type):
+            answers, _, _ = result
+            ips = []
+            for answer in answers:
+                if record_type == dns.A and isinstance(answer.payload, dns.Record_A):
+                    ips.append(answer.payload.dottedQuad())
+                elif record_type == dns.AAAA and isinstance(answer.payload, dns.Record_AAAA):
+                    if isinstance(answer.payload.address, bytes):
+                        ip6_bytes = answer.payload.address
+                        if len(ip6_bytes) == 16:
+                            ip_str = socket.inet_ntop(socket.AF_INET6, ip6_bytes)
+                            ips.append(ip_str)
+                        else:
+                            print(f"Invalid IPv6 bytes length: {len(ip6_bytes)}")
+                    else:
+                        ips.append(str(answer.payload.address))
+            return ips
+
+        resolver = client.getResolver()
+
+        # query A record (ipv4)
+        d_a = resolver.query(dns.Query(hostname, dns.A, dns.IN))
+        d_a_ips = d_a.addCallback(lambda res: extract_ips(res, dns.A))
+
+        # query AAAA record (ipv6)
+        d_aaaa = resolver.query(dns.Query(hostname, dns.AAAA, dns.IN))
+        d_aaaa_ips = d_aaaa.addCallback(lambda res: extract_ips(res, dns.AAAA))
+
+        # gather A record and AAAA record
+        return defer.gatherResults([d_a_ips, d_aaaa_ips], consumeErrors=True) \
+                .addCallback(lambda results: results[0] + results[1])
+        
+    @inlineCallbacks
+    def add_meta_server(self, meta_addr):
+        if not isinstance(meta_addr, str):
+            logger.error("meta_addr must be a string: %s", meta_addr)
+            returnValue(False)
+        
+        try:
+            host, port_str = meta_addr.split(':', 1)
+        except ValueError:
+            logger.error("invalid address format (expected host:port): %s", meta_addr)
+            returnValue(False)
+
+        ips = []
+        if self.is_valid_ip(host):
+            ips = [host]
+        else:
+            try:
+                ips = yield self.resolve_all_ips(host)
+                hp = host_port()
+                if hp.from_string(meta_addr):
+                    hp.type = self.get_host_type(ips)
+                    self.host_ports.append(hp)
+                    logger.info("resolved hostname %s to IP type:%s, addr:%s", hp.host, hp.type, ips)
+            except Exception as e:
+                logger.error("failed to resolve hostname %s: %s", host, e)
+                returnValue(False)
+            
+        for ip in ips:
+            self.addr_list.append((ip, int(port_str)))
+
+        returnValue(True)
+
+    @inlineCallbacks
+    def resolve_host(self, err):
+        if not self.host_ports:
+            returnValue(None) 
+        
+        new_addr_list = []
+        for host_port in self.host_ports:
+            host, port = host_port.to_host_port()
+            try:
+                ips = yield self.resolve_all_ips(host)
+                host_port.type = self.get_host_type(ips)
+                for ip in ips:
+                    new_addr_list.append((ip, port))
+                logger.info("resolved hostname %s to IP type:%s, addr:%s", host_port.host, host_port.type, ips)
+            except Exception as e:
+                    logger.error("failed to resolve hostname %s: %s", host, e)
+                    continue

Review Comment:
   Inconsistent indentation: line 373 has excessive indentation (20 spaces instead of 16). This should be aligned with the exception block above it.
   ```suggestion
                   logger.error("failed to resolve hostname %s: %s", host, e)
                   continue
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -641,23 +781,34 @@ def __init__(self, meta_addrs=None, table_name='',
         self.name = table_name
         self.table = Table(table_name, self, timeout)
         self.meta_session_manager = MetaSessionManager(table_name, timeout)
-        if isinstance(meta_addrs, list):
-            for meta_addr in meta_addrs:
-                self.meta_session_manager.add_meta_server(meta_addr)
+        self.initial_meta = False
+        self.meta_addrs = meta_addrs if isinstance(meta_addrs, list) else []
         PegasusHash.populate_table()
         self.timeout_times = 0
         self.update_partition = False
         self.timer = reactor.callLater(META_CHECK_INTERVAL, self.check_state)
 
+    @inlineCallbacks
     def init(self):
         """
         Initialize the client before you can use it.
 
         :return: (DeferredList) True when initialized succeed, others when failed.
         """
+        if not self.initial_meta:
+            deferreds = []
+            self.initial_meta = True
+            for host_port in self.meta_addrs:
+                d = self.meta_session_manager.add_meta_server(host_port)
+                deferreds.append(d)
+            results = yield defer.DeferredList(deferreds, consumeErrors=True)
+            if not any(success for success, _ in results):
+                raise Exception("all meta servers failed to initialize")

Review Comment:
   The error message 'all meta servers failed to initialize' doesn't provide actionable information. Consider including details about what addresses failed or the specific errors encountered to aid troubleshooting.
   ```suggestion
                   # Collect details about failed meta servers
                   failed_details = []
                   for idx, (success, result) in enumerate(results):
                       if not success:
                           addr = self.meta_addrs[idx] if idx < len(self.meta_addrs) else "<unknown>"
                           error_str = str(result)
                           failed_details.append(f"{addr}: {error_str}")
                   details_msg = "; ".join(failed_details) if failed_details else "No details available"
                   raise Exception(f"all meta servers failed to initialize. Details: {details_msg}")
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#issuecomment-3517838914

   @WJSGDBZ Could you please add some simple tests, and fix the conflicts


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#discussion_r2895091003


##########
python-client/pypegasus/pgclient.py:
##########
@@ -253,20 +256,159 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            ipaddress.ip_address(address)
+            return True
+        except ValueError:
+            return False
+            
+    def get_host_type(self, ip_list):
+        """
+        Determines the host type based on the provided list of IP addresses.
+        
+        This function analyzes the IP version distribution in the given list to classify
+        the host as IPv4-only, IPv6-only, mixed (both IPv4 and IPv6), or invalid.
+        """
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup

Review Comment:
   On the server side, `kHostTypeGroup` means that the `host_port` class stores a list of addresses. However, in the Python client it is defined as a mix of IPv4 and IPv6, which is inconsistent with the server-side definition. What is the rationale behind this design?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#discussion_r2514974424


##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False
+            
+    def get_host_type(self, ip_list):
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup
+        elif has_ipv4:
+            return host_port_types.kHostTypeIpv4
+        elif has_ipv6:
+            return host_port_types.kHostTypeIpv6
+        else:
+            return host_port_types.kHostTypeInvalid
+    
+    def resolve_all_ips(self, hostname):

Review Comment:
   Missing docstrings for newly added methods. Add docstrings for `get_host_type`, `resolve_all_ips`, and `resolve_host` methods explaining their purpose, parameters, and return values. This is especially important for complex asynchronous methods that perform DNS resolution.



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False

Review Comment:
   Code duplication and inconsistency: The code uses both `socket.inet_pton()` in `is_valid_ip` (lines 256, 261) and `ipaddress.ip_address()` in `get_host_type` (line 272) to validate IP addresses. Consider using a consistent approach throughout. The `ipaddress` module is more Pythonic and provides better error handling. Consider refactoring `is_valid_ip` to use `ipaddress.ip_address()` instead.
   ```suggestion
               ipaddress.ip_address(address)
               return True
           except ValueError:
               return False
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False
+            
+    def get_host_type(self, ip_list):
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup
+        elif has_ipv4:
+            return host_port_types.kHostTypeIpv4
+        elif has_ipv6:
+            return host_port_types.kHostTypeIpv6
+        else:
+            return host_port_types.kHostTypeInvalid
+    
+    def resolve_all_ips(self, hostname):
+
+        def extract_ips(result, record_type):
+            answers, _, _ = result
+            ips = []
+            for answer in answers:
+                if record_type == dns.A and isinstance(answer.payload, dns.Record_A):
+                    ips.append(answer.payload.dottedQuad())
+                elif record_type == dns.AAAA and isinstance(answer.payload, dns.Record_AAAA):
+                    if isinstance(answer.payload.address, bytes):
+                        ip6_bytes = answer.payload.address
+                        if len(ip6_bytes) == 16:
+                            ip_str = socket.inet_ntop(socket.AF_INET6, ip6_bytes)
+                            ips.append(ip_str)
+                        else:
+                            logger.error('Invalid IPv6 bytes length: %s', len(ip6_bytes))
+                    else:
+                        ips.append(str(answer.payload.address))
+            return ips
+
+        resolver = client.getResolver()
+
+        # query A record (ipv4)
+        d_a = resolver.query(dns.Query(hostname, dns.A, dns.IN))
+        d_a_ips = d_a.addCallback(lambda res: extract_ips(res, dns.A))
+
+        # query AAAA record (ipv6)
+        d_aaaa = resolver.query(dns.Query(hostname, dns.AAAA, dns.IN))
+        d_aaaa_ips = d_aaaa.addCallback(lambda res: extract_ips(res, dns.AAAA))
+
+        # gather A record and AAAA record
+        return defer.gatherResults([d_a_ips, d_aaaa_ips], consumeErrors=True) \
+                .addCallback(lambda results: results[0] + results[1])
+        
+    @inlineCallbacks
+    def add_meta_server(self, meta_addr):
+        if not isinstance(meta_addr, str):
+            logger.error("meta_addr must be a string: %s", meta_addr)
+            returnValue(False)
+        
+        try:
+            host, port_str = meta_addr.split(':', 1)
+        except ValueError:
+            logger.error("invalid address format (expected host:port): %s", meta_addr)
+            returnValue(False)
+
+        ips = []
+        if self.is_valid_ip(host):
+            ips = [host]
+        else:
+            try:
+                ips = yield self.resolve_all_ips(host)
+                hp = host_port()
+                if hp.from_string(meta_addr):
+                    hp.type = self.get_host_type(ips)
+                    self.host_ports.append(hp)
+                    logger.info("resolved hostname %s to IP type:%s, addr:%s", hp.host, hp.type, ips)
+            except Exception as e:
+                logger.error("failed to resolve hostname %s: %s", host, e)
+                returnValue(False)
+            
+        for ip in ips:
+            self.addr_list.append((ip, int(port_str)))
+
+        returnValue(True)
 
-            ip, port = ip_port[0], int(ip_port[1])
-            self.addr_list.append((ip, port))
+    @inlineCallbacks
+    def resolve_host(self, err):
+        if not self.host_ports:
+            returnValue(None) 
+        
+        new_addr_list = []
+        for host_port in self.host_ports:
+            host, port = host_port.to_host_port()
+            try:
+                ips = yield self.resolve_all_ips(host)
+                host_port.type = self.get_host_type(ips)
+                for ip in ips:
+                    new_addr_list.append((ip, port))
+                logger.info("resolved hostname %s to IP type:%s, addr:%s", host_port.host, host_port.type, ips)
+            except Exception as e:
+                logger.error("failed to resolve hostname %s: %s", host, e)
+                continue
+                
+        if not new_addr_list or new_addr_list == self.addr_list:
+            returnValue(None)
+     
+        stale_sessions = []
+        for rpc_addr in list(self.session_dict):
+            ip, port = rpc_addr.to_ip_port()
+            if (ip, port) not in new_addr_list:
+                stale_sessions.append(self.session_dict.pop(rpc_addr))
+                logger.info("removed stale server: %s:%s", ip, port)
 
-            return True
-        else:
-            return False
+        self.addr_list = new_addr_list
+
+        for session in stale_sessions:
+            if session: 

Review Comment:
   The `session` parameter at line 386 is checked with `if session:` but session objects may have custom `__bool__` implementations. Use `if session is not None:` for more explicit and reliable checking. Additionally, using `reactor.callFromThread` suggests potential thread safety issues - ensure that session closing is thread-safe or document why `callFromThread` is necessary here.
   ```suggestion
               if session is not None:
                   # Ensure session.close is called in the reactor thread for thread safety.
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False
+            
+    def get_host_type(self, ip_list):
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup
+        elif has_ipv4:
+            return host_port_types.kHostTypeIpv4
+        elif has_ipv6:
+            return host_port_types.kHostTypeIpv6
+        else:
+            return host_port_types.kHostTypeInvalid
+    
+    def resolve_all_ips(self, hostname):
+
+        def extract_ips(result, record_type):
+            answers, _, _ = result
+            ips = []
+            for answer in answers:
+                if record_type == dns.A and isinstance(answer.payload, dns.Record_A):
+                    ips.append(answer.payload.dottedQuad())
+                elif record_type == dns.AAAA and isinstance(answer.payload, dns.Record_AAAA):
+                    if isinstance(answer.payload.address, bytes):
+                        ip6_bytes = answer.payload.address
+                        if len(ip6_bytes) == 16:
+                            ip_str = socket.inet_ntop(socket.AF_INET6, ip6_bytes)
+                            ips.append(ip_str)
+                        else:
+                            logger.error('Invalid IPv6 bytes length: %s', len(ip6_bytes))
+                    else:
+                        ips.append(str(answer.payload.address))
+            return ips
+
+        resolver = client.getResolver()
+
+        # query A record (ipv4)
+        d_a = resolver.query(dns.Query(hostname, dns.A, dns.IN))
+        d_a_ips = d_a.addCallback(lambda res: extract_ips(res, dns.A))
+
+        # query AAAA record (ipv6)
+        d_aaaa = resolver.query(dns.Query(hostname, dns.AAAA, dns.IN))
+        d_aaaa_ips = d_aaaa.addCallback(lambda res: extract_ips(res, dns.AAAA))
+
+        # gather A record and AAAA record
+        return defer.gatherResults([d_a_ips, d_aaaa_ips], consumeErrors=True) \
+                .addCallback(lambda results: results[0] + results[1])
+        
+    @inlineCallbacks
+    def add_meta_server(self, meta_addr):
+        if not isinstance(meta_addr, str):
+            logger.error("meta_addr must be a string: %s", meta_addr)
+            returnValue(False)
+        
+        try:
+            host, port_str = meta_addr.split(':', 1)
+        except ValueError:
+            logger.error("invalid address format (expected host:port): %s", meta_addr)
+            returnValue(False)
+
+        ips = []
+        if self.is_valid_ip(host):
+            ips = [host]
+        else:
+            try:
+                ips = yield self.resolve_all_ips(host)
+                hp = host_port()
+                if hp.from_string(meta_addr):
+                    hp.type = self.get_host_type(ips)
+                    self.host_ports.append(hp)
+                    logger.info("resolved hostname %s to IP type:%s, addr:%s", hp.host, hp.type, ips)
+            except Exception as e:
+                logger.error("failed to resolve hostname %s: %s", host, e)
+                returnValue(False)
+            
+        for ip in ips:
+            self.addr_list.append((ip, int(port_str)))
+
+        returnValue(True)
 
-            ip, port = ip_port[0], int(ip_port[1])
-            self.addr_list.append((ip, port))
+    @inlineCallbacks
+    def resolve_host(self, err):

Review Comment:
   The `err` parameter in `resolve_host` is declared but never used. If this parameter is intended to receive error information from the errback chain, document its purpose and consider logging or handling the error. If it's not needed, consider removing it from the function signature.
   ```suggestion
       def resolve_host(self):
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:

Review Comment:
   The variable `e` in the exception handler is unused. Consider either removing it (`except Exception:`) or using it in the error handling logic if additional debugging information is needed.
   ```suggestion
           except Exception:
               # maybe ipv6
               try:
                   socket.inet_pton(socket.AF_INET6, address)
                   return True
               except Exception:
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:

Review Comment:
   The variable `e` in the exception handler is unused. Consider either removing it (`except Exception:`) or using it in the error handling logic.
   ```suggestion
               except Exception:
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)

Review Comment:
   Missing import for the `socket` module. While `socket` is imported in `pypegasus.base.ttypes`, relying on transitive imports through wildcard imports (`from pypegasus.base.ttypes import *`) is fragile and not a best practice. Add an explicit `import socket` at the top of the file.



##########
python-client/pypegasus/pgclient.py:
##########
@@ -245,20 +247,146 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
-
-    def add_meta_server(self, meta_addr):
-        rpc_addr = rpc_address()
-        if rpc_addr.from_string(meta_addr):
-            ip_port = meta_addr.split(':')
-            if not len(ip_port) == 2:
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            socket.inet_pton(socket.AF_INET, address)
+            return True
+        except Exception as e:
+            # maybe ipv6
+            try:
+                socket.inet_pton(socket.AF_INET6, address)
+                return True
+            except Exception as e:
                 return False
+            
+    def get_host_type(self, ip_list):
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup
+        elif has_ipv4:
+            return host_port_types.kHostTypeIpv4
+        elif has_ipv6:
+            return host_port_types.kHostTypeIpv6
+        else:
+            return host_port_types.kHostTypeInvalid
+    
+    def resolve_all_ips(self, hostname):
+
+        def extract_ips(result, record_type):
+            answers, _, _ = result
+            ips = []
+            for answer in answers:
+                if record_type == dns.A and isinstance(answer.payload, dns.Record_A):
+                    ips.append(answer.payload.dottedQuad())
+                elif record_type == dns.AAAA and isinstance(answer.payload, dns.Record_AAAA):
+                    if isinstance(answer.payload.address, bytes):
+                        ip6_bytes = answer.payload.address
+                        if len(ip6_bytes) == 16:
+                            ip_str = socket.inet_ntop(socket.AF_INET6, ip6_bytes)
+                            ips.append(ip_str)
+                        else:
+                            logger.error('Invalid IPv6 bytes length: %s', len(ip6_bytes))
+                    else:
+                        ips.append(str(answer.payload.address))
+            return ips
+
+        resolver = client.getResolver()
+
+        # query A record (ipv4)
+        d_a = resolver.query(dns.Query(hostname, dns.A, dns.IN))
+        d_a_ips = d_a.addCallback(lambda res: extract_ips(res, dns.A))
+
+        # query AAAA record (ipv6)
+        d_aaaa = resolver.query(dns.Query(hostname, dns.AAAA, dns.IN))
+        d_aaaa_ips = d_aaaa.addCallback(lambda res: extract_ips(res, dns.AAAA))
+
+        # gather A record and AAAA record
+        return defer.gatherResults([d_a_ips, d_aaaa_ips], consumeErrors=True) \
+                .addCallback(lambda results: results[0] + results[1])
+        
+    @inlineCallbacks
+    def add_meta_server(self, meta_addr):
+        if not isinstance(meta_addr, str):
+            logger.error("meta_addr must be a string: %s", meta_addr)
+            returnValue(False)
+        
+        try:
+            host, port_str = meta_addr.split(':', 1)
+        except ValueError:
+            logger.error("invalid address format (expected host:port): %s", meta_addr)
+            returnValue(False)
+
+        ips = []
+        if self.is_valid_ip(host):
+            ips = [host]
+        else:
+            try:
+                ips = yield self.resolve_all_ips(host)
+                hp = host_port()
+                if hp.from_string(meta_addr):
+                    hp.type = self.get_host_type(ips)
+                    self.host_ports.append(hp)
+                    logger.info("resolved hostname %s to IP type:%s, addr:%s", hp.host, hp.type, ips)
+            except Exception as e:
+                logger.error("failed to resolve hostname %s: %s", host, e)
+                returnValue(False)
+            
+        for ip in ips:
+            self.addr_list.append((ip, int(port_str)))
+
+        returnValue(True)
 
-            ip, port = ip_port[0], int(ip_port[1])
-            self.addr_list.append((ip, port))
+    @inlineCallbacks
+    def resolve_host(self, err):
+        if not self.host_ports:
+            returnValue(None) 
+        
+        new_addr_list = []
+        for host_port in self.host_ports:
+            host, port = host_port.to_host_port()
+            try:
+                ips = yield self.resolve_all_ips(host)
+                host_port.type = self.get_host_type(ips)
+                for ip in ips:
+                    new_addr_list.append((ip, port))
+                logger.info("resolved hostname %s to IP type:%s, addr:%s", host_port.host, host_port.type, ips)
+            except Exception as e:
+                logger.error("failed to resolve hostname %s: %s", host, e)
+                continue
+                
+        if not new_addr_list or new_addr_list == self.addr_list:

Review Comment:
   Potential logic issue: The comparison `new_addr_list == self.addr_list` at line 373 compares lists of tuples. However, the order of addresses might differ even if the same servers are present, leading to unnecessary re-initialization. Consider using set comparison (`set(new_addr_list) == set(self.addr_list)`) to check if the addresses have actually changed, regardless of order.
   ```suggestion
           if not new_addr_list or set(new_addr_list) == set(self.addr_list):
   ```



##########
python-client/pypegasus/pgclient.py:
##########
@@ -21,12 +21,14 @@
 import os
 import logging.config
 import six
+import ipaddress
 
 from thrift.Thrift import TMessageType, TApplicationException
 from twisted.internet import defer
 from twisted.internet import reactor
-from twisted.internet.defer import inlineCallbacks, succeed, fail
+from twisted.internet.defer import inlineCallbacks, succeed, fail, returnValue

Review Comment:
   Import of 'succeed' is not used.
   Import of 'fail' is not used.
   ```suggestion
   from twisted.internet.defer import inlineCallbacks, returnValue
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "WJSGDBZ (via GitHub)" <gi...@apache.org>.
WJSGDBZ commented on PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#issuecomment-3584002768

   Sure, I've already added the test process above.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "WJSGDBZ (via GitHub)" <gi...@apache.org>.
WJSGDBZ commented on PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#issuecomment-3690728211

   > > Sure, I've already added the test process above.
   > 
   > The conflict is still exist :(
   
   Thanks for the reminder, the conflict has been resolved.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#issuecomment-3679070531

   > Sure, I've already added the test process above.
   
   The conflict is still exist :(


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


Re: [PR] feat(Python-client):add dns resolution support [incubator-pegasus]

Posted by "WJSGDBZ (via GitHub)" <gi...@apache.org>.
WJSGDBZ commented on code in PR #2306:
URL: https://github.com/apache/incubator-pegasus/pull/2306#discussion_r2929725387


##########
python-client/pypegasus/pgclient.py:
##########
@@ -253,20 +256,159 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            ipaddress.ip_address(address)
+            return True
+        except ValueError:
+            return False
+            
+    def get_host_type(self, ip_list):
+        """
+        Determines the host type based on the provided list of IP addresses.
+        
+        This function analyzes the IP version distribution in the given list to classify
+        the host as IPv4-only, IPv6-only, mixed (both IPv4 and IPv6), or invalid.
+        """
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup

Review Comment:
   already fix this to be compatible with the server



##########
python-client/pypegasus/pgclient.py:
##########
@@ -253,20 +256,159 @@ class MetaSessionManager(SessionManager):
     def __init__(self, table_name, timeout):
         SessionManager.__init__(self, table_name, timeout)
         self.addr_list = []
+        self.host_ports = []
+        self.query_times = 0
+        
+    # validate if the given string is a valid IP address     
+    def is_valid_ip(self, address):
+        try:
+            ipaddress.ip_address(address)
+            return True
+        except ValueError:
+            return False
+            
+    def get_host_type(self, ip_list):
+        """
+        Determines the host type based on the provided list of IP addresses.
+        
+        This function analyzes the IP version distribution in the given list to classify
+        the host as IPv4-only, IPv6-only, mixed (both IPv4 and IPv6), or invalid.
+        """
+        has_ipv4 = False
+        has_ipv6 = False
+
+        for ip_str in ip_list:
+            try:
+                ip = ipaddress.ip_address(ip_str)
+                if ip.version == 4:
+                    has_ipv4 = True
+                elif ip.version == 6:
+                    has_ipv6 = True
+            except ValueError:
+                continue 
+
+        if has_ipv4 and has_ipv6:
+            return host_port_types.kHostTypeGroup

Review Comment:
   @empiredan already fix this to be compatible with the server



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org