-
Notifications
You must be signed in to change notification settings - Fork 419
Do not look for a port in a Unix socket domain path #470
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.
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
Conversation
A gentle nudge, just to see if there is life on this project? :) |
asyncpg/connect_utils.py
Outdated
if hostspec.startswith('/'): | ||
hosts.append(hostspec) | ||
# keep sequence ordering | ||
hostlist_ports.append(5432) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. Socket paths still have to take the port into consideration. Use port or default_port[i]
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well default_port
at that index might not exist, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The length of the default_port
list is guaranteed to be the same as the number of specified hosts. See _validate_port_spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply replace
addr, _, hostspec_port = hostspec.partition(':')
with
if not hostspec.startswith('/'):
addr, _, hostspec_port = hostspec.partition(':')
else:
addr = hostspec
hostspec_port = ''
Happy to squash if that makes the review simpler. Not sure your preferred process. |
Please squash. Thanks! |
Contributes to MagicStack#419 Signed-off-by: Sylvain Hellegouarch <[email protected]>
Hi @elprans, I can appreciate you are quite busy but any visibility on a potential new release? Just to plan accordingly :) |
Hi @Lawouach, I'll make a release next week. |
Woo ooh! Awesome! Thanks. |
fantastic! |
Contributes to #419
Signed-off-by: Sylvain Hellegouarch [email protected]