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

Net::HTTP::Persistent adapter #92

Merged
merged 1 commit into from
Jul 3, 2013
Merged

Conversation

mynameisrufus
Copy link
Contributor

🍰

Using the url of the request to generate the thread pool key.
@rubiii
Copy link
Contributor

rubiii commented Jun 1, 2013

thanks @mynameisrufus. i'm not sure if the current design of HTTPI (creating a new adapter per request) plays well with the net/http persistent library.

@rogerleite can you take care of this?

@rogerleite
Copy link
Member

@rubiii, @mynameisrufus did a clever tricky using host as thread key ... frontfoot@2d00774#L4R35

On May i changed my job and i'm trying to find some time. We have this and excon adapter #91 to review.

@rubiii
Copy link
Contributor

rubiii commented Jun 1, 2013

@rogerleite ahh i see. hit me up when you have some time.

@mynameisrufus
Copy link
Contributor Author

net net-http-persistent uses a pool of connections in a thread based on a key https://github.com/drbrain/net-http-persistent/blob/master/lib/net/http/persistent.rb#L594

I guess my approach would fall down if example.com/A and example.com/B ended up at different servers?

@rogerleite
Copy link
Member

sorry @mynameisrufus i don't get it. Why would fall down?

@mynameisrufus
Copy link
Contributor Author

No your right, it would be fine.

@rogerleite rogerleite merged commit 2d00774 into savonrb:master Jul 3, 2013
@rogerleite
Copy link
Member

@mynameisrufus thks! This adapter will be available at release v2.1.0

@rubiii rubiii mentioned this pull request Jul 20, 2013
27 tasks
@rubiii
Copy link
Contributor

rubiii commented Jul 22, 2013

releaed with version 2.1.0.

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

Successfully merging this pull request may close these issues.

3 participants