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

Ensure that each thread has its own client #591

Merged
merged 1 commit into from
Oct 12, 2017
Merged

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe
cc @stripe/api-libraries

Implements the fix suggested by @jurriaan in #590. I don't have a lot of experience with threads in Ruby but this seems reasonable.

(Had to update .rubocop_todo.yml because the new test caused a Metrics/ClassLength offense. We might want to exclude test code from those metrics cops.)

@brandur-stripe
Copy link
Contributor

brandur-stripe commented Oct 12, 2017

Thanks Olivier!

I think there is also a chance that we should think about moving to something besides Faraday on a future major release — as @jurriaan mentioned, it doesn't do connection pooling by default (or even easily), and this is really unfortunate given that a lot of people aren't going to think to turn it on. We probably have a huge number of Europe-based users who'd hugely benefit from a few tweaks in this area.

For now though, this makes sense.

@jurriaan
Copy link

This is awesome! Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants