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(pg connection-manager.js): add query_timeout
to dialogOptions
#13476
Conversation
This option has existed in pg since v7.7 - see discussion at brianc/node-postgres#1760 Adding to properties to be passed to new connection config so it can be used with sequelize
query_timeout
to dialogOptions
missed comma
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.
missing comma
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.
Looks safe enough. Ideally there'd be tests for it, but this should be fine.
// Client side request timeout in milliseconds. Added in pg v7.7 | ||
'query_timeout', |
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.
Took me a moment to realize this isn't a sequelize option, its an option pg
. Can you add a link to the docs for clarification? Thanks.
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.
I think the majority if not all of those dialectOptions are related to the postgres client (there is a comment at the very top of that code block)
Same as #13258 ? |
going to close this one since the other PR has a test |
Pull Request check-list
npm run test
ornpm run test-DIALECT
pass with this change (including linting)?Description of change
The
query_timeout
option has existed in pg since v7.7 - see discussion at brianc/node-postgres#1760This change adds it to properties to be passed to new connection config so it can be used with sequelize