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: support forever option #54

Merged
merged 3 commits into from
Jul 19, 2019
Merged

feat: support forever option #54

merged 3 commits into from
Jul 19, 2019

Conversation

callmehiphop
Copy link
Contributor

Closes #48

So admittedly this only adds support for non-proxy users. However my goal is to resolve googleapis/nodejs-bigquery#41, which appears to mainly affect GCF users, so I think its ok to not include proxy support?

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 17, 2019
@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #54 into master will increase coverage by 0.91%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   62.78%   63.69%   +0.91%     
==========================================
  Files           1        1              
  Lines         309      314       +5     
  Branches       23       25       +2     
==========================================
+ Hits          194      200       +6     
+ Misses        114      113       -1     
  Partials        1        1
Impacted Files Coverage Δ
src/index.ts 63.69% <90.9%> (+0.91%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6409e2...27e1763. Read the comment docs.

@callmehiphop
Copy link
Contributor Author

@JustinBeckwith took a shot at some tests and modified the implementation to set the agent in the response request object (request module does this). As a result I had to shuffle some types around, but I think its ok? PTAL!

teenyRequest({uri, forever: true}, (err, res) => {
assert.ifError(err);
// tslint:disable-next-line no-any
assert.strictEqual((res.request.agent as any).keepAlive, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, it's weird that you had to cast this. What's up with that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Agent type is missing a number of properties :[

src/index.ts Outdated Show resolved Hide resolved
@JustinBeckwith JustinBeckwith merged commit 746d70e into googleapis:master Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support forever option reuse connections on table.insert
3 participants