-
Notifications
You must be signed in to change notification settings - Fork 259
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
Added option.maxTries #184
base: master
Are you sure you want to change the base?
Added option.maxTries #184
Conversation
PR seems to be missing tests. I would really need this feature for This feature would allow us to decide if we want to wait only in that case if @coopernurse If this PR is fixed is will it be merged? |
Setting this option stops trying to create resource after specified consequent failures. Prevents pool going into infinite loop if resource can not be created.
59889c4
to
02ab9aa
Compare
I have re-based my pull request against latest version. |
Added autotest for maxTries.
I've added tests for maxTries feature. |
Is it hard to merge this PR? We desperately need in ability to stop attempts to create resource (attach to database in our case) after first try failed. |
Is it possible to use the existing
to achieve the same outcome, or is that sufficiently overcomplicated to pipe together? |
Consider we are pooling connections to db server. Somehow wrong login/password were specified. Current behavior is that the library will try to reconnect again and again... Correct work in this case would be only one attempt and then throw exception to a calling code. Now, to emulate this we do first connection without pooling. If it succeeds then we disconnect and run through pool library. If it fails we analyze return value and throw appropriate exception. |
Callback
|
@gsbelarus is it your own code which is using node-pool or 3rd party library? |
our own.
…On Thu, Jun 27, 2019 at 5:49 PM Mikael Lepistö ***@***.***> wrote:
@gsbelarus <https://github.com/gsbelarus> is it your own code which is
using node-pool or 3rd party library?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#184?email_source=notifications&email_token=ABHPTVG3XM5LBYZTNPAY6BLP4THODA5CNFSM4DDGGNGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYXLRDY#issuecomment-506378383>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABHPTVEOI4HCLFS7ASY3UXLP4THODANCNFSM4DDGGNGA>
.
|
We ended up writing our own pool implementation which hat that error propagation feature in addition to various timeout configurations to be used with knex. You migth want to take a look https://github.com/vincit/tarn.js That pool was written for knex, so it will probably not get any additional features in the future, but those current features are really robust. |
Thanks! Will give it a try. |
Please resolve the conflicts and I will be happy to merge this! |
Setting this option stops trying to create resource after specified
consequent failures. Prevents pool going into infinite loop if resource
can not be created.