Skip to content

support the tuple ip and port #1283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wuzhongdehua wants to merge 1 commit into
base: trunk
Choose a base branch
from
+10 −1
Open

support the tuple ip and port #1283

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion cassandra/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

@absurdfarce absurdfarce Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

log.debug("PANDU: translated address {}".format(translated_address))
Copy link
Contributor

@absurdfarce absurdfarce Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug line should be removed

if type(translated_address) is tuple and len(translated_address)>1:
Copy link
Contributor

@absurdfarce absurdfarce Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ip = translated_address[0]
port = translated_address[1]
log.debug( "PANDU: CREATING Default endpoint {} {}".format(ip, port))
Copy link
Contributor

@absurdfarce absurdfarce Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This debug line should also be removed

else:
ip = translated_address
return DefaultEndPoint(
self.cluster.address_translator.translate(addr),
ip,
port)


Expand Down