Conversation
This is a port of GoogleCloudPlatform/cloud-sql-proxy#1381. Fixes #132.
| } | ||
|
|
||
| const readmeText = ` | ||
| When applications attempt to open files in this directory, a remote connection |
There was a problem hiding this comment.
I updated the README for AlloyDB.
| return c.NewInode(ctx, &readme{}, fs.StableAttr{}), fs.OK | ||
| } | ||
|
|
||
| instanceURI, err := toFullURI(instance) |
There was a problem hiding this comment.
This check expands the requested instance (e.g., proj.region.cluster.inst) into a full instance URI. If instance isn't a valid short Unix name, it returns an error.
hessjcg
left a comment
hessjcg
left a comment
There was a problem hiding this comment.
Please double-check the concurrency in the Client.Close() method.
hessjcg
left a comment
hessjcg
left a comment
There was a problem hiding this comment.
Good to go. A few nits remain.
| @@ -151,10 +160,13 @@ var ( | |||
| // 'projects/<PROJECT>/locations/<REGION>/clusters/<CLUSTER>/instances/<INSTANCE>' | |||
| // Additionally, we have to support legacy "domain-scoped" projects (e.g. "google.com:PROJECT") | |||
| instURIRegex = regexp.MustCompile("projects/([^:]+(:[^:]+)?)/locations/([^:]+)/clusters/([^:]+)/instances/([^:]+)") | |||
There was a problem hiding this comment.
Optional Nit: If you wanted to be really clever, use a non-capturing group (?: ) for the sub-domain in PROJECT group
instURIRegex = regexp.MustCompile(
"projects/([^:]+(?::[^:]+)?)/locations/([^:]+)/clusters/([^:]+)/instances/([^:]+)")
Then the so that your code down below goes 1,2,3,4:
project := string(m[1])
region := string(m[2])
cluster := string(m[3])
name := string(m[4])
There was a problem hiding this comment.
Fancy -- I'm working on making this code exported from the connector and will revisit this change with unit tests when I get there.
hessjcg
left a comment
hessjcg
left a comment
There was a problem hiding this comment.
Looks good. Nice cleanup.
2 participants
This is a port of GoogleCloudPlatform/cloud-sql-proxy#1381.
Fixes #132.