Netflix / ribbon

Ribbon is a Inter Process Communication (remote procedure calls) library with built in software load balancers. The primary usage model involves REST calls with various serialization scheme support.
Apache License 2.0
4.55k stars 1.24k forks source link

Apache HttpClient that extends AbstractLoadBalancerAwareClient #55

Open yairogen opened 10 years ago

yairogen commented 10 years ago

I am looking into create a LoadBalancerAwareClient that uses Apache Client.

I've found NFHttpClient but it doesn't do what I need.

Is there such an implementation?

Would you like me to add it as a patch?

Not clear from documentation what is the purpose of NFHttpClient in the first place.

allenxwang commented 10 years ago

The class that extends AbstractLoadBalancerAwareClient is com.netflix.niws.client.http.RestClient. RestClient uses NFHttpClient as the underlying client for HTTP communication.

yairogen commented 10 years ago

Thanks. I thought it uses the Jersey client, no? On Oct 30, 2013 6:04 PM, "allenxwang" notifications@github.com wrote:

The class that extends 'AbstractLoadBalancerAwareClient' is com.netflix.niws.client.http.RestClient. RestClient uses NFHttpClient as the underlying client for HTTP communication.

— Reply to this email directly or view it on GitHubhttps://github.com/Netflix/ribbon/issues/55#issuecomment-27404292 .

allenxwang commented 10 years ago

Yes, it uses Jersey client. Jersey client in turn uses the Apache HttpClient.

Are you suggesting that you would like an implementation directly on top of HttpClient without Jersey client?

yairogen commented 10 years ago

Exactly. Jersey client as a rest client is a stopper for us. Non rest APIs don't feel natural. On Oct 30, 2013 8:02 PM, "allenxwang" notifications@github.com wrote:

Yes, it uses Jersey client. Jersey client in turn uses the Apache HttpClient.

Are you suggesting that you would like an implementation directly on top of HttpClient without Jersey client?

— Reply to this email directly or view it on GitHubhttps://github.com/Netflix/ribbon/issues/55#issuecomment-27419824 .

allenxwang commented 10 years ago

Sounds good. I would like to see your pull request.

A couple of questions/suggestions:

yairogen commented 10 years ago

Nice.

So, if I'm picking this up then the AsyncClient should also have a LoadBalancerAwareClient wrapping, right?

allenxwang commented 10 years ago

For async client, once you implement the interface, you can wrap it by AsyncLoadBalancingClient and it will be able to do load balancing. AsyncLoadBalancingClient does not extend LoadBalancerAwareClient but it gets certain functions from LoadBalancerContext.

For sync client, currently you need to extend AbstractLoadBalancerAwareClient.

yairogen commented 10 years ago

Allen, I'm happy to contribute. Can you point me to some resource on the process of contribution? Also, I'm not a Gradle user. I use maven. Can this work out somehow?

allenxwang commented 10 years ago

See https://github.com/Netflix/ribbon/wiki/How-to-contribute

It is very easy to build with gradle once you have forked the repo. It should build out of the box without any set up.

build.gradle on the top level defines all the dependencies. settings.gradle contains the set of sub projects to build. You should only need to work with those two files if you need to add a sub project.

yairogen commented 8 years ago

@allenxwang I haven't had the time to contribute but am now interested again. Was this already done by someone?