Skip to content
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

feat: add ability to set per-request requestTimeout #1530

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dhensby
Copy link
Collaborator

@dhensby dhensby commented Aug 16, 2023

What this does:

This adds an optional config object that can be passed to Request, Transaction and PreparedStatement objects to provide one-off/per-instance overrides for config settings on the pool. At the moment that is purely for requestTimeout, but does provide the foundation to add other options too.

Things to think about:

  1. Should Transaction classes accept this option given that it is non-trivial in tedious to actually set the timeout on the begin/end/commit transaction queries (because tedious creates their own request without exposing it when starting/commiting transactions)? If we don't we end up with a bit of a mis-matched API and perhaps we should add it, but it is then applied to a transactions requests unless overridden again...

Still to do:

  • Write tests
  • Add docs to Readme
  • Add support for transaction requests (or not?!)
  • Finish jsdocs
  • update typings (after merge/release)

Related issues:

Closes #1529

Pre/Post merge checklist:

  • Update change log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Request.prototype.setTimeout()
1 participant