Skip to content

Conversation

@shubha-rajan
Copy link
Contributor

fixes #1697

@shubha-rajan shubha-rajan requested a review from kurtisvg as a code owner April 26, 2020 22:56
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 26, 2020
@fhinkel
Copy link
Contributor

fhinkel commented Apr 27, 2020

Please update the PR title to match our commit message convention.

@shubha-rajan shubha-rajan changed the title Fix unclear configuration in postgres connection sample fix: update unclear configuration in postgres connection sample Apr 27, 2020
@shubha-rajan shubha-rajan changed the title fix: update unclear configuration in postgres connection sample fix(cloud-sql): update unclear configuration in postgres connection sample Apr 27, 2020
// [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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@shubha-rajan shubha-rajan requested a review from kurtisvg April 27, 2020 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cloud-sql knex.js related example configuration is inconsistent/confusing

4 participants