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
Add support for connecting to a Postgres database via Unix sockets. #70
Conversation
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.
Looks great, just some gardening comments 👍
extension PostgreSQLConnection { | ||
/// Connects to a Redis server using a TCP socket. | ||
public static func connect( | ||
hostname: String = "localhost", |
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.
There needs to be a deprecated method for this old API
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.
Added one.
extension PostgreSQLConnection { | ||
/// Connects to a PostgreSQL server using a TCP socket. | ||
public static func connect( | ||
to serverAddress: PostgreSQLDatabaseConfig.ServerAddress = .default, |
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.
There appears to be a formatting issue 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.
Done, thanks! Fixed whitespace everywhere.
case unixSocket(path: String) | ||
|
||
public static let `default` = ServerAddress.tcp(hostname: "localhost", port: 5432) | ||
public static let defaultViaSocket = ServerAddress.unixSocket(path: "/tmp/.s.PGSQL.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.
I think socketDefault
would be a fine name 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.
Renamed.
/// Destination port. | ||
public let port: Int | ||
|
||
public enum ServerAddress { |
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 deserves a comment.
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.
Added.
PTAL |
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.
great, thanks!
Hey @MrMage, you just merged a pull request, have a coin! You now have 9 coins. |
Happy to add a connectivity test as well, but I don't know whether the Postgres DB used in CI is set up for supporting Unix sockets. Let me know if you can provide any insights on that.
Motivation: Unix sockets have lower latency and higher throughput than TCP sockets. There are reports that indicate this to be particularly relevant when using Google Cloud SQL proxy.