Skip to content

New HttpClient for each new request #17

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.

Sign up for GitHub

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

Jump to bottom
Closed
nillis opened this issue Nov 22, 2016 · 5 comments
Closed

New HttpClient for each new request #17

nillis opened this issue Nov 22, 2016 · 5 comments
Labels
status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap

Comments

@nillis
Copy link
Contributor

nillis commented Nov 22, 2016

Issue Summary

Is there a specific reason why a new HttpClient is instantiated on each request which is made through this client?

Reusing a HttpClient instance is considered a best practice regarding to performance and memory pressure. I'm glad to do a pull request, but first wanted to verify there isn't a specific reason to not reuse the instance.
f.e. https://skimming.net/why-you-should-reuse-httpclient-instances/

image
@thinkingserious
Copy link
Contributor

No reason.

A pull request would be greatly appreciated :)

@nillis nillis mentioned this issue Nov 23, 2016
Merged
@chriskallen
Copy link

Is this going to be merged in anytime soon?

@thinkingserious thinkingserious added status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap labels Feb 21, 2017
@thinkingserious
Copy link
Contributor

Hi @chriskallen,

I'm not sure, we are awaiting a signed CLA from @nillis.

I noticed we have the same issue in our new v9 SDK, so I've created a ticket for that. I would appreciate you vote on that issue to help it move up the queue. Thanks!

@nillis
Copy link
Contributor Author

nillis commented Feb 24, 2017

I submitted my CLA so my PR can be merged :)

@thinkingserious
Copy link
Contributor

This issue was solved here: #18

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@thinkingserious @chriskallen @nillis