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: [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]
