-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix(cloud-sql): update unclear configuration in postgres connection sample #1746
fix(cloud-sql): update unclear configuration in postgres connection sample #1746
Conversation
Please update the PR title to match our commit message convention. |
cloud-sql/postgres/knex/server.js
Outdated
// [START cloud_sql_postgres_knex_lifetime] | ||
// 'idleTimeoutMillis' is the number of milliseconds a connection must sit idle in the pool | ||
// and not be checked out before it is automatically closed. | ||
knex.client.pool.idleTimeoutMillis = 600000; // 10 minutes |
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.
Same as the other PR - similar arguments are in different places for other samples. Can we move this into connection count and see if there is a "max lifetime" parameter?
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 isn't a max lifetime parameter (knex uses the same underlying pool implementation as mssql) so we'll probably have to leave this sample blank like in the mysql sample.
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.
SGTM - make sure when we update the "Connecting to Cloud SQL" page we include a similar line for SQL Server.
fixes #1697