You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "wuzhongdehua (via GitHub)" <gi...@apache.org> on 2026/03/23 10:07:40 UTC
[PR] support the tuple ip and port [cassandra-python-driver]
wuzhongdehua opened a new pull request, #1283:
URL: https://github.com/apache/cassandra-python-driver/pull/1283
(no comment)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] support the tuple ip and port [cassandra-python-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1283:
URL: https://github.com/apache/cassandra-python-driver/pull/1283#issuecomment-4113675527
@bschoening I'm curious what you think about this one.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] support the tuple ip and port [cassandra-python-driver]
Posted by "bschoening (via GitHub)" <gi...@apache.org>.
bschoening commented on PR #1283:
URL: https://github.com/apache/cassandra-python-driver/pull/1283#issuecomment-4113756751
@wuzhongdehua can you add a description to this PR? I don't understand why a tuple format is preferable and turns one line into ten.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] support the tuple ip and port [cassandra-python-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1283:
URL: https://github.com/apache/cassandra-python-driver/pull/1283#discussion_r2977493762
##########
cassandra/connection.py:
##########
@@ -237,18 +236,27 @@ def create(self, row):
# create the endpoint with the translated address
# TODO next major, create a TranslatedEndPoint type
+ ## Overriden by Pandu. using ip + port translation
+ translated_address = self.cluster.address_translator.translate(addr)
+ log.debug("PANDU: translated address {}".format(translated_address))
+ if type(translated_address) is tuple and len(translated_address)>1:
+ ip = translated_address[0]
+ port = translated_address[1]
+ log.debug( "PANDU: CREATING Default endpoint {} {}".format(ip, port))
Review Comment:
This debug line should also be removed
##########
cassandra/connection.py:
##########
@@ -237,18 +236,27 @@ def create(self, row):
# create the endpoint with the translated address
# TODO next major, create a TranslatedEndPoint type
+ ## Overriden by Pandu. using ip + port translation
+ translated_address = self.cluster.address_translator.translate(addr)
+ log.debug("PANDU: translated address {}".format(translated_address))
Review Comment:
This debug line should be removed
##########
cassandra/connection.py:
##########
@@ -237,18 +236,27 @@ def create(self, row):
# create the endpoint with the translated address
# TODO next major, create a TranslatedEndPoint type
+ ## Overriden by Pandu. using ip + port translation
+ translated_address = self.cluster.address_translator.translate(addr)
Review Comment:
It seems really odd to me that this changes the API from address -> translated address to address -> (translated address, translated port). It's not immediately obvious to me how many use cases would be able to compute a translation for port numbers based only on an input address. Seems like the more complete option would be to provide both address and port if we're going to return a combination of translated address and port.
This also probably helps out with the API. We have something like this right now:
```
def translate(self, addr) -> str:
```
The proposal here is to move to something like this:
```
def translate(self, addr) -> (str | [tuple[str,int])
```
This is an API change but it's an addition rather than a change of existing functionality; we won't break any address translators out there. But if we add a new method which clearly returns a host/string tuple (of some form) then we can clearly distinguish between these cases:
```
def translate(self, addr) -> str:
...
def translate_all(self, addr, port) -> tuple[str,int]
```
We'd still have the question of how we want DefaultEndPointFactory to use these methods... but it _might_ be clearer if we did so?
##########
cassandra/connection.py:
##########
@@ -237,18 +236,27 @@ def create(self, row):
# create the endpoint with the translated address
# TODO next major, create a TranslatedEndPoint type
+ ## Overriden by Pandu. using ip + port translation
+ translated_address = self.cluster.address_translator.translate(addr)
+ log.debug("PANDU: translated address {}".format(translated_address))
+ if type(translated_address) is tuple and len(translated_address)>1:
Review Comment:
I think I'd prefer a namedtuple here rather than a straight tuple... just to avoid any possible confusion with positions in a base tuple.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] support the tuple ip and port [cassandra-python-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1283:
URL: https://github.com/apache/cassandra-python-driver/pull/1283#issuecomment-4784009910
I'm going to close this one out for now. If you would like to re-open @wuzhongdehua we'd be happy to continue the conversation above... just let us know!
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] support the tuple ip and port [cassandra-python-driver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1283:
URL: https://github.com/apache/cassandra-python-driver/pull/1283#discussion_r3356124128
##########
cassandra/connection.py:
##########
@@ -236,8 +236,17 @@ def create(self, row):
# create the endpoint with the translated address
# TODO next major, create a TranslatedEndPoint type
+ ## Overriden by Pandu. using ip + port translation
+ translated_address = self.cluster.address_translator.translate(addr)
Review Comment:
This change introduces new behavior (supporting `(ip, port)` return values from `AddressTranslator.translate()`), but there are existing unit/integration tests around endpoint creation and address translation. Please add a test that exercises a translator returning `(translated_ip, translated_port)` to ensure `DefaultEndPointFactory.create()` uses the overridden port and still works with the discovered port when a plain address is returned.
##########
cassandra/connection.py:
##########
@@ -236,8 +236,17 @@ def create(self, row):
# create the endpoint with the translated address
# TODO next major, create a TranslatedEndPoint type
+ ## Overriden by Pandu. using ip + port translation
+ translated_address = self.cluster.address_translator.translate(addr)
+ log.debug("PANDU: translated address {}".format(translated_address))
+ if type(translated_address) is tuple and len(translated_address)>1:
+ ip = translated_address[0]
+ port = translated_address[1]
+ log.debug( "PANDU: CREATING Default endpoint {} {}".format(ip, port))
+ else:
+ ip = translated_address
Review Comment:
`type(translated_address) is tuple and len(translated_address) > 1` is too permissive/fragile: it will silently accept tuples longer than 2, and it will pass a 1-item tuple through as the endpoint address (breaking the expectation that `DefaultEndPoint.address` is a string). Prefer `isinstance(..., tuple)` and explicitly validate/unpack a 2-tuple `(ip, port)`.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] support the tuple ip and port [cassandra-python-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce closed pull request #1283: support the tuple ip and port
URL: https://github.com/apache/cassandra-python-driver/pull/1283
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org