fix(cloud-sql): Updated sample to include full JDBC url in comments.#3562
Conversation
|
|
||
| // Configure which instance and what database user to connect with. | ||
| // Equivalent URL: | ||
| // jdbc:mysql:///<DATABASE_NAME>?cloudSqlInstance=<INSTANCE_CONNECTION_NAME>&socketFactory=com.google.cloud.sql.mysql.SocketFactory&user=<MYSQL_USER_NAME>&password=<MYSQL_USER_PASSWORD> |
There was a problem hiding this comment.
that's a long line, and doesn't actually represent what you have below.
|
|
||
| // Configure which instance and what database user to connect with. | ||
| // Equivalent URL: | ||
| // jdbc:postgresql:///<DATABASE_NAME>?cloudSqlInstance=<INSTANCE_CONNECTION_NAME>&socketFactory=com.google.cloud.sql.postgres.SocketFactory&user=<POSTGRESQL_USER_NAME>&password=<POSTGRESQL_USER_PASSWORD> |
lesv
left a comment
There was a problem hiding this comment.
Perhaps you can explain how to use that line and break it up.
|
I moved the URL to after the data source property for socket factory was set. It's still really long though. The issue is that we're constructing that URL step by step, but I'm not sure if users realize that the partial URL we have in the sample is for connections that use the socket factory. |
|
I would actually prefer the line to be above. The reason I asked for this change was because someone didn't read the code and tried to copy L54 without the additional parameters, and filed feedback when it didn't work. I wanted to give a sneak peak of the full JDBC url in advance for the inpatient folks. I'm also okay if we opt to just link to this section of the README for the full JDBC configurations: https://github.com/GoogleCloudPlatform/cloud-sql-jdbc-socket-factory#creating-the-jdbc-url As long as it's next to the base url I'm happy. |
|
I'll leave it to your judgement. |
|
@kurtisvg moved it back to before the base URL. I'll merge after you approve |
Fixes internal bug b/165099138.