Copilot commented on code in PR #1253:
URL:
https://github.com/apache/cassandra-python-driver/pull/1253#discussion_r2620724065
##########
tests/integration/standard/test_policies.py:
##########
@@ -80,7 +80,22 @@ def test_only_connects_to_subset(self):
only_connect_hosts = {"127.0.0.1", "127.0.0.2"}
white_list =
ExecutionProfile(load_balancing_policy=WhiteListRoundRobinPolicy(only_connect_hosts))
cluster = TestCluster(execution_profiles={"white_list": white_list})
- #cluster =
Cluster(load_balancing_policy=WhiteListRoundRobinPolicy(only_connect_hosts))
+ session = cluster.connect(wait_for_all_pools=True)
+ queried_hosts = set()
+ for _ in range(10):
+ response = session.execute('SELECT * from system.local',
execution_profile="white_list")
+ queried_hosts.update(response.response_future.attempted_hosts)
+ queried_hosts = set(host.address for host in queried_hosts)
+ self.assertEqual(queried_hosts, only_connect_hosts)
+
+ @local
+ def test_only_connects_to_subset_with_port(self):
+ only_connect_hosts = {
+ ("127.0.0.1", 9042),
+ ("127.0.0.2", 9043)
Review Comment:
The test specifies port 9043 for IP 127.0.0.2, but based on typical
Cassandra cluster setups where each node uses the same port (usually 9042),
this may not match the actual port configuration. Verify that the test cluster
actually has a node at 127.0.0.2:9043, otherwise this test will fail or not
properly validate the port-specific filtering.
```suggestion
("127.0.0.2", 9042)
```
##########
cassandra/policies.py:
##########
@@ -410,43 +410,62 @@ class WhiteListRoundRobinPolicy(RoundRobinPolicy):
regardless of what datacenter the nodes may be in, but
only if that node exists in the list of allowed nodes
- This policy is addresses the issue described in
+ This policy addresses the issue described in
https://datastax-oss.atlassian.net/browse/JAVA-145
Where connection errors occur when connection
attempts are made to private IP addresses remotely
"""
def __init__(self, hosts):
"""
- The `hosts` parameter should be a sequence of hosts to permit
- connections to.
+ The `hosts` parameter should be one of:
+ * A sequence of str containing the addresses to permit connections to.
+ * A sequence of (str, int) containing the address + port to permit
connections to.
"""
self._allowed_hosts = tuple(hosts)
- self._allowed_hosts_resolved = [endpoint[4][0] for a in
self._allowed_hosts
- for endpoint in socket.getaddrinfo(a,
None, socket.AF_UNSPEC, socket.SOCK_STREAM)]
+
+ self._allowed_endpoints = []
+ self._allowed_addresses = []
+
+ for host in hosts:
+ if isinstance(host, str):
+ # If the user provides only a host name, the whitelist should
check only the port, do not assume they use the default port.
Review Comment:
The comment states 'check only the port' but should say 'check only the
address' since when only a hostname is provided, the policy checks addresses
without port filtering.
```suggestion
# If the user provides only a host name, the whitelist
should check only the address, not the port.
```
##########
tests/unit/test_policies.py:
##########
@@ -1252,6 +1252,17 @@ def test_hosts_with_hostname(self):
self.assertEqual(policy.distance(host), HostDistance.LOCAL)
+ def test_hosts_with_hostname_and_port(self):
+ hosts = ['localhost']
+ policy = WhiteListRoundRobinPolicy(hosts)
+ host = Host(DefaultEndPoint("127.0.0.1", 1000), SimpleConvictionPolicy)
+ policy.populate(None, [host])
+
+ qplan = list(policy.make_query_plan())
+ self.assertEqual(sorted(qplan), [host])
+
+ self.assertEqual(policy.distance(host), HostDistance.LOCAL)
Review Comment:
The test name suggests it tests hostname-and-port functionality, but it only
passes a hostname string ('localhost') without a port. This test should either
be renamed to reflect hostname-only testing or should pass a (hostname, port)
tuple to test the new port-aware functionality. Consider adding a separate test
that uses `[('localhost', 1000)]` to properly test the port-specific whitelist
feature.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
