feat: agent pooling #86
Conversation
googlebot
added
the
cla: yes
|
Working on tests, but throwing it out there for early feedback! |
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 69.35% 72.68% +3.32%
==========================================
Files 1 2 +1
Lines 359 399 +40
Branches 36 41 +5
==========================================
+ Hits 249 290 +41
+ Misses 109 108 -1
Partials 1 1
Continue to review full report at Codecov.
|
|
@bcoe @JustinBeckwith @stephenplusplus I think I'm ready for a review here whenever some one gets a chance! |
| if (!pool.has(key)) { | ||
| // tslint:disable-next-line variable-name | ||
| const Agent = isHttp ? HTTPAgent : HTTPSAgent; | ||
| pool.set(key, new Agent({keepAlive: true})); |
There was a problem hiding this comment.
if I"m reading this correctly, we only cache the agent if reqOpts.forever is true; should this logic be outside of the check for whether the forever key is to be appended.
There was a problem hiding this comment.
I'm definitely open to that idea, do you have anything specific in mind?
| if (proxy) { | ||
| // tslint:disable-next-line variable-name | ||
| const Agent = isHttp | ||
| ? require('http-proxy-agent') |
There was a problem hiding this comment.
is there any reason we couldn't cache the proxy agent?
There was a problem hiding this comment.
Last time I suggested this, I think you told us node caches requires under the hood 🤣
There was a problem hiding this comment.
oh, I meant in the Map being used for the pool, we return immediately if a proxy is in use.
There was a problem hiding this comment.
Well, we could, but I'm not sure if we need to. My understanding for keepAlive (or in request world forever) is that it will pool the underlying sockets and re-use them. Currently neither of the proxy agents we use support such a feature so I don't know if pooling the proxy agents makes sense.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #84 🦕