curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sean Busbey <bus...@cloudera.com>
Subject Re: Proposal : remove references to guava library from public APIs
Date Wed, 01 Apr 2015 15:40:17 GMT
This thread started with Simon running in to a Guava problem while trying
to use Curator.

On Wed, Apr 1, 2015 at 10:33 AM, Jordan Zimmerman <
jordan@jordanzimmerman.com> wrote:

> I know Guava has caused problems elsewhere. But, I don’t recall any
> problems with Curator.
>
> -JZ
>
>
>
> On April 1, 2015 at 10:32:04 AM, Mike Drob (madrob@cloudera.com) wrote:
>
> Respectfully disagree. Guava issues have plagued Hadoop in the past (and to
> an extent, still do). Guava versions tend to iterate relatively quickly and
> don't always have the keenest eye on backwards compatibility. When you
> expose your guava implementation, that locks all of your users into the
> same version because newer versions may no longer work with your library
> (an issue which osgi seeks to address).
>
> The JDK, on the other hand, goes to painstaking lengths to ensure backwards
> compat for the past 20+ years.
>
> On Wed, Apr 1, 2015 at 9:55 AM, Jordan Zimmerman <
> jordan@jordanzimmerman.com
> > wrote:
>
> > I consider Guava to be part of the JDK so I disagree. We haven’t had many
> > issues with Guava compatibility. In fact, I can’t think of one Jira
> > reported on it. So, my vote would be to leave things as they are.
> >
> > -JZ
> >
> >
> >
> > On April 1, 2015 at 3:09:49 AM, Simon Kitching (
> > simon.kitching@smartstream-stp.com) wrote:
> >
> > Thanks Jordan.
> >
> > The root cause of the problem isn't really anything osgi-specific; it's
> > the fact that curator uses another library (guava) as part of its
> _public_
> > API.
> >
> > Imagine you wanted to change from using guava to some other collections
> > library - it wouldn't be possible without breaking the public API of
> > curator. The question is whether guava really should be part of the
> curator
> > API, or should be just an implementation detail. I would suggest that the
> > use of guava is really an implementation detail that should be
> > private/hidden - unlike use of jaxws for example, which really is an
> > externally-defined abstract API and is reasonable to include as part of
> the
> > public API of curator.
> >
> > This difference between used-in-the-impl and used-in-the-api doesn't
> > matter so much in a java application with one big classloader that has
> > every single jarfile in it; if you need the guava library in the
> classpath
> > for internal use by curator, then it will automatically also be visible
> to
> > all other classes and so it is impossible to have a different version of
> > the library also present. Using OSGi (which creates multiple
> classloaders)
> > can allow multiple versions of the same lib - but only when the lib is
> only
> > used-in-the-impl (ie is for "internal" usage by a jarfile).
> >
> > Re keeping a compatible API: possibly all classes in package
> > "org.apache.curator.framework.listen" could be copied into a new package,
> > and then the ListenerContainer class updated to not expose guava. All
> > classes in "org.apache.curator.framework.listen" could then be
> deprecated.
> > As long as OSGi code avoids using any code from the old package, there
> > would be no binding to the guava library used by curator. I don't know if
> > that would be better than simply changing the API for a couple of methods
> > or not.
> >
> > I will create a JIRA issue to update the maven-build-plugin version;
> > that's trivial and is not a binary incompatibility.
> >
> > Unless somebody objects within the next few days, I will also create a
> > JIRA issue regarding the APIs that expose guava. I might have time to
> work
> > on this myself next month (may).
> >
> > By the way, if you're interested in how OSGi classloading works, this may
> > be helpful: http://moi.vonos.net/java/osgi-classloaders/
> >
> > Thanks & Regards,
> > Simon (aka skitching at apache.org)
> >
> > ________________________________
> > From: Jordan Zimmerman [jordan@jordanzimmerman.com]
> > Sent: Tuesday, March 31, 2015 17:46
> > To: dev@curator.apache.org; Simon Kitching
> > Subject: Re: Proposal : remove references to guava library from public
> APIs
> >
> > I don’t have an objection in general. The biggest problem for me is that
> I
> > know very little about OSGI. All of the OSGI work has been contributed so
> > it’s hard to make sure that we keep it working. That said, changing
> > existing APIs is very disruptive to the Curator community. I’d like to
> see
> > someone (Simon?) commit to maintaining the OSGi compatibility of Curator
> > and make sure releases don’t introduce issues. Also, can the existing
> APIs
> > remain and new, OSGi compatible parallel APIs be added?
> >
> > -JZ
> >
> >
> >
> > On March 31, 2015 at 7:39:08 AM, Simon Kitching (
> > simon.kitching@smartstream-stp.com<mailto:
> > simon.kitching@smartstream-stp.com>) wrote:
> >
> > Hi,
> >
> > I've noticed that several curator classes expose the use of classes from
> > google's guava library [1] as part of their *public* api.
> >
> > [1] maven artifact "com.google.guava:guava" which contains java packages
> > com.google.common.*
> >
> > In an OSGi environment, it is possible to load multiple different
> versions
> > of the same library, as long as that library is a purely internal
> > implementation detail. Unfortunately, as curator exposes its use of
> guava,
> > this makes it impossible for code that uses curator to also use a
> different
> > version of Guava for its own purposes. Unfortunately, this has just
> bitten
> > me : I need to write code that uses both curator (requires guava 16.0 or
> > later) and a third-party library that requires an earlier version of
> guava.
> >
> > Are there any objections to me raising an enhancement issue in JIRA for
> > this? Note that this change would be a binary incompatibility (though
> > fairly limited).
> >
> > The problem classes that I have found are:
> > * curator-framework:
> org.apache.curator.framework.listen.ListenerContainer
> > : method forEach takes a parameter of type
> com.google.common.base.Function
> > * curator-framework:
> > org.apache.curator.framework.api.transaction.CuratorTransactionResult :
> > method ofTypeAndPath returns com.google.common.base.Predicate
> > * curator-x-discovery-server:
> > org.apache.curator.x.discovery.server.contexts.GenericDiscoveryContext :
> > constructor takes param of type com.google.common.reflect.TypeToken
> > * curator-x-discovery: org.apache.curator.x.discovery.InstanceFilter :
> > inherits from com.google.common.base.Predicate
> >
> > And by the way, I noticed that org.codehaus.jackson types are also used
> in
> > public APIs (at least, GenericDiscoveryContext). It may also be worth
> > looking into whether it is really necessary to expose this dependency.
> >
> > The goal of the change would be to ensure that in the MANIFEST.MF file
> for
> > each curator bundle (jarfile), the Export-Packages line minimises the
> > "uses:=" entries which refer to non-curator packages. A uses-constraint
> on
> > a package should only be needed when something in the package being
> > exported uses an external type in its public API.
> >
> > As a separate problem, I have noticed that with the 2.7.1 release (at
> > least), the "bnd" tool (via maven-bundle-plugin) is adding entries to the
> > "uses" entries even when the referenced library is purely used
> internally.
> > I have found a reference (https://github.com/emlun/bnd-uses-strange)
> that
> > suggests this is a bug which is fixed in later bnd releases.
> Unfortunately
> > I can find no release-notes for bnd, nor any source-code repository so
> > cannot confirm this. However updating curator/pom.xml to specify the
> > following fixes the "uses" clauses:
> > <maven-bundle-plugin-version>2.5.3</maven-bundle-plugin-version>
> >
> > Thanks & Regards,
> > Simon
> >
> > ________________________________
> > The information in this email is confidential and may be legally
> > privileged. It is intended solely for the addressee. Access to this email
> > by anyone else is unauthorised. If you are not the intended recipient,
> any
> > disclosure, copying, distribution or any action taken or omitted to be
> > taken in reliance on it, is prohibited and may be unlawful. SmartStream
> > Technologies GmbH, Vienna Twin Tower, Wienerbergstrasse 11, 1100 Vienna,
> > Austria, FN 194340w, HG Wien
> > ________________________________
> > The information in this email is confidential and may be legally
> > privileged. It is intended solely for the addressee. Access to this email
> > by anyone else is unauthorised. If you are not the intended recipient,
> any
> > disclosure, copying, distribution or any action taken or omitted to be
> > taken in reliance on it, is prohibited and may be unlawful. SmartStream
> > Technologies GmbH, Vienna Twin Tower, Wienerbergstrasse 11, 1100 Vienna,
> > Austria, FN 194340w, HG Wien
> >
>



-- 
Sean

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message