-
Notifications
You must be signed in to change notification settings - Fork 350
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
feat: Add unix-socket-path to instance command arguments. #1623
Conversation
17dfaba
to
0f39052
Compare
internal/proxy/proxy.go
Outdated
// connected to the Cloud SQL instance. If set, takes precedence over Addr | ||
// and Port. | ||
// connected to the Cloud SQL instance. The full path to the socket will be | ||
// UnixSocket + os.PathSeparator + Name. If set, takes precedence over Addr and Port. |
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.
super nit: wrap this and the docs below at 80 chars. Editors usually have a way to do this automatically so you don't have to manually move stuff around.
internal/proxy/proxy.go
Outdated
address = inst.UnixSocketPath | ||
|
||
// If UnixSocketPath ends .s.PGSQL.5432, remove it for consistency | ||
if strings.HasPrefix(version, "POSTGRES") && path.Base(address) == ".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.
What happens if "address" doesn't exist?
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.
We return an error on line 803 if the parent directory for address
doesn't exist.
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.
Minor rewrite of the docs.
cmd/root.go
Outdated
@@ -172,6 +172,17 @@ Instance Level Configuration | |||
my-project:us-central1:my-db-server \ | |||
'my-project:us-central1:my-other-server?address=0.0.0.0&port=7000' | |||
|
|||
Instance Level Configuration Parameters: |
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.
Let's leave this section off and immediately go into the particular parameter here. For example,
When necessary, you may specify the full path to a Unix socket using the unix-socket-path
query parameter. The value of the parameter should be an absolute path to where you want the socket to be created. Parent directories must exist, or the socket creation will fail. For Postgres instances, the proxy will ensure that the last path element is '.s.PGSQL.5432', appending it if necessary. For example,
./cloud-sql-proxy
'my-project:us-central1:my-db-server?unix-socket-path=/path/to/socket'
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.
Updated with a few minor edits.
cmd/root.go
Outdated
@@ -172,6 +172,16 @@ Instance Level Configuration | |||
my-project:us-central1:my-db-server \ | |||
'my-project:us-central1:my-other-server?address=0.0.0.0&port=7000' | |||
|
|||
When necessary, you may specify the full path to a Unix socket. Set the | |||
unix-socket-path query parameter to the absolute path of the unix socket for |
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.
s/unix/Unix/
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.
fixed.
cmd/root.go
Outdated
the database instance. The parent directory of the unix-socket-path must | ||
exist when the proxy starts or else socket creation will fail. For Postgres | ||
instances, the proxy will ensure that the last path element is | ||
'.s.PGSQL.5432' appending it if necessary. For example, |
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.
let's get rid of the whitespace 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.
fixed.
9ccec19
to
29981d2
Compare
Change Description
This adds a new parameter
unix-socket-path
to instances to specify the precise unix socket path.For example, this command line:
Will open a unix socket to the database at the path
/var/cloudsql/mydb
.This is different than the existing parameter
unix-socket
which automatically appends the instance name to the path.Checklist
Relevant issues: