curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Drob <mad...@cloudera.com>
Subject Re: Proposal : remove references to guava library from public APIs
Date Wed, 01 Apr 2015 22:24:50 GMT
It sounds like we need a JIRA to look at the implementation and figure out
how disruptive this change actually is. I created
https://issues.apache.org/jira/browse/CURATOR-200 to investigate.

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

> Sorry if I’m not being clear. I don’t mind changing the APIs. My only
> concern is disrupting the current community. I’m hoping that we can add new
> APIs and deprecate the old ones. I don’t know if OSGi supports this or not.
>
> -JZ
>
>
>
> On April 1, 2015 at 11:08:09 AM, Simon Kitching (
> simon.kitching@smartstream-stp.com) wrote:
>
> Hi All,
>
> This problem would have occurred with or without OSGi. However without
> OSGi, it wouldn't be solvable at all :-)
>
> I need to write code that:
> * uses a library that needs a very old version of guava, ie is not
> compatible with guava 16.0.1.
> * uses curator which relies on guava 16.0.1 or later.
>
> With a "flat" classpath, this just cannot be solved. With OSGi, it can be
> solved _if_ one or both of these libraries only relies "internally" on
> guava.
>
> If curator had used guava everywhere throughout its public API, I wouldn't
> bother asking here. But curator is _very close_ to merely using guava as an
> internal detail, and so it seemed worth asking if the dev team were
> interested in making the few changes necessary.
>
> Note that this problem is partially self-inflicted due to my employer's
> requirement to use this ugly library that relies on very old guava
> (actually, "com.google.collections:google-collections" which is the earlier
> name for guava and sadly guava uses the same com.google.common.*
> package-names). However every library makes incompatible changes from time
> to time, so this will probably also bite someone else sometime.
>
> Thanks for at least discussing this. As there's apparently no great
> enthusiasm for the idea, I'll need to solve my immediate problem some other
> way (eg by forking curator locally or using maven-shade-plugin or similar)
> for now. I'll try to find time next month (May) to see if I can come up
> with some patches that might be acceptable (as mentioned in "compatible
> API" below).
>
> Regards,
> Simon
>
> ________________________________
> From: Jordan Zimmerman [jordan@jordanzimmerman.com]
> Sent: Wednesday, April 01, 2015 17:47
> To: dev@curator.apache.org; Sean Busbey
> Cc: Mike Drob; Simon Kitching
> Subject: Re: Proposal : remove references to guava library from public APIs
>
> The problem, though, was with OSGi. OSGi support for Curator was added by
> Curator users and not the core team. Curator has never committed to OSGi
> support. Of course, we could change that.
>
> -JZ
>
>
>
>
> On April 1, 2015 at 10:41:51 AM, Sean Busbey (busbey@cloudera.com<mailto:
> busbey@cloudera.com>) wrote:
>
> 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
> ________________________________
> 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
>

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