-
-
Notifications
You must be signed in to change notification settings - Fork 630
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
Replace throttle implementation by external library #654
Conversation
- got from 11.8.6 to 13.0.0 - mocha from 9.1.4 to 10.2.0 - ramda from 0.21.0 to 0.29.0 - and minor bumps on others
# Conflicts: # package-lock.json # package.json
Don't mind about the third commit description, i missed a copy paste. it contains eslint adjustments. |
lib/utils/processPages.js
Outdated
}); | ||
|
||
if (opts.throttle !== undefined) { | ||
const throttle = pThrottle({ |
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.
why did you need to add this explicit throttle setup here? doesn't the setup in the internal request module apply here as well by just passing the throttle option?
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 thougth the same, but test fails if i don't edit processpage when it batch request fulldetails
I wrote this implementation const req = (resolve, reject) => requestLib(opts)
.then((response) => resolve(response.body))
.catch((error) => reject(error));
let promiseToResolve = (resolve, reject) => req(resolve, reject);
if (throttleParams !== undefined) {
const throttle = pThrottle({
limit: throttleParams.limit ? throttleParams.limit : 1000,
interval: throttleParams.interval ? throttleParams.interval : 0
});
promiseToResolve = (resolve, reject) => throttle(req(resolve, reject));
}
return new Promise(promiseToResolve); But we could use the throttling method everytime with default values set to {limit: 1000, interval : 0} const throttle = pThrottle({
limit: throttleParams && throttleParams.limit ? throttleParams.limit : 1000,
interval: throttleParams && throttleParams.interval ? throttleParams.interval : 0
});
return new Promise((resolve, reject) => throttle(requestLib(opts)
.then((response) => resolve(response.body))
.catch((error) => reject(error)))); |
Both the That's why i force interval or limit with default values if one is missing |
so a couple of things:
|
in fact, p-throttle doesnt queue requests, i'll try to use throttledQueue instead |
closing due to inactivity, but feel free to send a new one. |
No description provided.