rickfast / consul-client

Java Client for Consul HTTP API
Other
571 stars 241 forks source link

Replace JAX-RS (and maybe Jackson) #93

Closed rickfast closed 8 years ago

rickfast commented 8 years ago

Originally using JAX-RS seemed like a good idea to allow JEE folks to use the implementation of their choice. In practice, it's been a pain in the ass to make this client work out of the box for all dependency configurations.

The Vault client I maintain uses retrofit, ok http, and GSON and is much more portable and easy to maintain.

Should be able to maintain the clients public API and swap out the innards.

jhovell commented 8 years ago

@rickfast interesting, I might be able to pitch in at some point. I agree, it can be a headache to have to make an entire app manage one Jackson/javax.ws.rs implementation. By the way, I was planning to include your library with a submission to https://github.com/wildfly-swarm/wildfly-swarm which is sort of like a Spring Boot competitor for JEE apps. I saw you had an Apache license, but just wanted to check with you to make sure you're OK with that.

Your Vault client isn't OSS (yet)?

rickfast commented 8 years ago

Totally okay with that :) The Vault Client is on my personal github. Totally OSS

mrwilby commented 8 years ago

+1 this would be wonderful. I am currently in dependency hell as I try to integrate this library in Netflix Exhibitor for zookeeper cluster formation (using consul as the backing coordinator). Exhibitor is using very old libraries including Jax RS 1.x.

rickfast commented 8 years ago

@mrwilby @jhovell I'll create the 1.0 branch which will include this work. I'll try to convert one client this week using Retrofit.

mrwilby commented 8 years ago

@rickfast Thank you. My java foo is not strong enough to resolve my dependency hell with Exhibitor so this would indeed be much appreciated.

gjesse commented 8 years ago

One of the reasons I started using & contributing some to this library is specifically that it used jax-rs and jackson, which is already part of the standard stack I'm using (dropWizard), so it was a big win, as opposed to some other competitor clients that used gson.

I don't want to hold back progress though, and have real sympathy for those stuck using older jax-rs implementations, so i would just advocate considering making the clients interfaces, and possibly splitting alternate implementations into modules, if it makes sense to do so. If it doesn't make sense then it's probably better to just make the library as self-contained as possible as you are planning to.

mrwilby commented 8 years ago

I agree that makes sense. I am a dropwizard user myself, so generally I would agree with @gjesse. But trying to retrofit this library into old codebases is tricky. I have maven-shaded to my wits end with my scenario (involving netflix exhibitor, as said) but still no luck.

Anyway, I would obviously not advocate for re-architecting the internals of any library just on my account.

jhovell commented 8 years ago

@rickfast yeah class conflicts inside a JVM is a major drawback of any single-classloader Java app. That said, I'm confused what is actually causing problems in your case jaxrs 1.0 is generally compatible with 2.0. I don't know Gradle that well, but in Maven it's fairly trivial (though tedious) to exclude JARs you don't want (some old crufty JAXRS 1.x impl) and provide something new (Apache CXF or just mark as provided). I would imagine this would clear up the issue with your Netflix library.

Moving to a less popular REST framework I suppose decreases your risk of running into conflicts as your project grows, but it's not really tackling what is really an inevitable problem.

Generally major versions get different package names so both can live on classpath happily and minor versions are backward compatible (just use the newest one).

Another alternative I have seen used is to create a shaded jar. I haven't actually written anything that requires it, but my understanding is that you use a tool to rename all the packages of a 3rd party lib to give them a unique namespace. As an example, I've noticed the AWS Java SDK has a shade of Apache HttpClient, presumably so none of their users will run into conflicts and be tempted to mess with it.

https://maven.apache.org/plugins/maven-shade-plugin/

Might be easier than "throwing out the baby with the bathwater"... there are good reasons to move to a more opinionated/modern framework like retrofit/swagger/feign/ok http/whatever but not sure if dependency hell is the best reason.

amirabiri commented 8 years ago

+1 for modules & interfaces.

I for example am using Commons http 4 (not sure if it's jaxed or if there is an adapter for it anywhere) with Jackson 2. I hate the idea of having yet another http client and json library bloating my deptree, perhaps even creating parallel thread and connection pools.

It should be possible to plug this library into a project using any kind of http client and json library, with the immediate suspects supported out of the box, and as simple interface as possible to implement if using something else.

rickfast commented 8 years ago

Implemented shaded JAR w/ retrofit. Changes are in 0.11.0. Functionality shouldn't have changed