Yeah we really can't add any dependencies for that client jar. Maybe we could move the perf test to the tools jar though?
-Jay On Tue, Jul 14, 2015 at 6:01 PM, Geoffrey Anderson <ge...@confluent.io> wrote: > Hi all, my pull request here: > > https://github.com/apache/kafka/pull/70/files#diff-59f3fe36571d1eee9f923df927a643eb > would > introduce a client-side dependency on the json-simple package. > > It is only used in a tool (VerifiableProducer.java), but it sounds like I > should probably use Jackson instead? > > Feel free to make comments on the pull request itself if this is too > tangential to the discussion. > > Thanks, > Geoff > > On Tue, Jul 14, 2015 at 5:23 PM, Jay Kreps <j...@confluent.io> wrote: > > > Ah, makes sense. Yes that addresses my concerns. > > > > -Jay > > > > On Tue, Jul 14, 2015 at 5:19 PM, Ismael Juma <ism...@juma.me.uk> wrote: > > > > > Hi Jay, > > > > > > Comments inline. > > > > > > On Tue, Jul 14, 2015 at 11:04 PM, Jay Kreps <j...@confluent.io> wrote: > > > > > > > Is this going to become a dependency for core and then transitively > for > > > the > > > > old clients? > > > > > > > > > That's right. > > > > > > > > > > The current json library is definitely not great, but it does > > > > parse json and it's not used in any context where performance is a > > > concern. > > > > > > > > > > The issue seemed to indicate that the blocking and slow performance > were > > > causing issues: > > > > > > https://issues.apache.org/jira/browse/KAFKA-1595 > > > > > > > > > > > > > > Because the older clients aren't well modularized, adding core > > > dependencies > > > > sucks these up into every user of the clients. This particularly > > becomes > > > a > > > > problem with common libraries since it will turn out we require > > version X > > > > but other code in the same app requires version Y. > > > > > > > > > > Yes, I understand. Still, if we use Jackson and only use methods that > > > existed in 2.0, then it's unlikely to cause issues. I checked it with > > > Jackson's author: https://twitter.com/ijuma/status/621106341503991808. > > > > > > The reasoning is as follows: > > > > > > - Jackson 1 and Jackson 2 use different packages, so they co-exist > > > peacefully. > > > - Jackson 2.x releases are backwards compatible so clients are free > to > > > choose whichever version they want. If they don't pick a version, > then > > > the > > > highest version among the dependencies will be chosen. > > > - It is possible that bugs in an untested release would cause > issues, > > > but that could affect the existing JSON parser too (clients may use > > > different Scala versions). > > > > > > Limiting ourselves to methods from Jackson 2.0 is not as hard as it > > sounds > > > because the code only interacts with our thin wrapper of Jackson, all > the > > > code that deals directly with Jackson is isolated. > > > > > > The new clients fix this issue but not everyone is using them yet. > > > > > > > > If there is a pressing need maybe we should just do it and people who > > > have > > > > problems can just hack their build to exclude the dependency (since > the > > > > client code won't need it). If not it might be better just to leave > it > > > for > > > > a bit until we have at least get a couple releases with both the new > > > > producer and the new consumer. > > > > > > > > > > Hacking the builds to exclude the transitive dependency would be a last > > > resort, but not needed in most (hopefully all) cases. > > > > > > Does this make it less problematic in your view? > > > > > > If people are concerned about this, we can delay it, of course. A bit > of > > a > > > shame though, as this change improves performance, makes the code more > > > readable and makes it safer. > > > > > > Ismael > > > > > >
