Jordan and Cameron I have double checked most of the projects I am aware and finally I came to the conclusion that upgrading to 5.0 in all of my cases is safe.
I only add a nuisance on Apache BookKeeper because we are building with -Werror and so using deprecated APIs is seen as an error, but it is not a binary compatibility issue. I have also run locally tests of the staged sources. I am casting my +1 I think that this discussion as been useful for the community Thank you Enrico Il giorno mer 27 mag 2020 alle ore 00:55 Jordan Zimmerman < jor...@jordanzimmerman.com> ha scritto: > If that comes to pass I'll write a new Tech Note on the wiki on how to > create a Compatibility JAR. > > -JZ > > > On May 26, 2020, at 5:41 PM, Cameron McKenzie <mckenzie....@gmail.com> > wrote: > > > > I will release on Sunday if we haven't got any feedback indicating we > > should do otherwise. > > > > On Wed, May 27, 2020 at 6:32 AM Enrico Olivelli <eolive...@gmail.com > <mailto:eolive...@gmail.com>> wrote: > > > >> > >> > >> Il giorno mar 26 mag 2020 alle ore 21:53 Jordan Zimmerman < > >> jor...@jordanzimmerman.com> ha scritto: > >> > >>> The initial email is almost a week old. I posted on Curator's Twitter > >>> account too. No response from anyone other than me Cameron and Enrico. > Very > >>> discouraging. Let's give it to the end of the week. If no one responds > I > >>> suggest we release and tell people how to work around it if they need > to. > >>> > >> > >> Agreed > >> > >> Enrico > >> > >> > >> > >>> > >>> -JZ > >>> > >>> On May 25, 2020, at 9:10 AM, Jordan Zimmerman < > jor...@jordanzimmerman.com> > >>> wrote: > >>> > >>> If most of the problem is about ListenerContainer, don't we have a way > to > >>> keep it and emulate it using and implementation based on the new API ? > >>> > >>> > >>> Unfortunately not. The issue is that it leaks a Guava class (Function) > in > >>> its API. See here: > >>> > https://github.com/apache/curator/blob/apache-curator-4.1.1/curator-framework/src/main/java/org/apache/curator/framework/listen/ListenerContainer.java#L87 > >>> > >>> -JZ > >>> > >>> On May 25, 2020, at 1:33 AM, Enrico Olivelli <eolive...@gmail.com> > wrote: > >>> > >>> Il giorno dom 24 mag 2020 alle ore 23:17 Jordan Zimmerman < > >>> jor...@jordanzimmerman.com> ha scritto: > >>> > >>> Enrico, > >>> > >>> It reminds me of the breaking changes in Guava and other widely used > >>> libraries. > >>> > >>> > >>> > >>> In fact Guava is terrible for people (like in my company) that deal > with > >>> lots of third party dependencies. > >>> > >>> > >>> The problem for us is that we can never change our APIs if this is the > >>> case. Note that ListenerContainer has been marked deprecated since > 4.1.1 ( > >>> > >>> > https://github.com/apache/curator/blob/apache-curator-4.1.1/curator-framework/src/main/java/org/apache/curator/framework/listen/ListenerContainer.java > >>> < > >>> > >>> > https://github.com/apache/curator/blob/apache-curator-4.1.1/curator-framework/src/main/java/org/apache/curator/framework/listen/ListenerContainer.java > >>> > >>> ). > >>> > >>> > >>> > >>> I didn't check the code yet, I am sorry, so maybe I saying something > that > >>> is not doable. > >>> If most of the problem is about ListenerContainer, don't we have a way > to > >>> keep it and emulate it using and implementation based on the new API ? > >>> > >>> as Jordan said, any other comment from the community will be very > >>> appreciated, maybe we are talking about smoke. > >>> > >>> Enrico > >>> > >>> > >>> > >>> So, we're really left with these options: > >>> > >>> Release Curator 5.0 and let the issues fall onto those with > compatibility > >>> problems > >>> Bundle or refer to a compatibility JAR that is put early in the > CLASSPATH > >>> as I outlined in my test project > >>> Move Curator 5.0 to a new package so that it can exist in the same JVM > as > >>> earlier versions of Curator. > >>> Backout the change and mark the APIs as deprecated and push the > problem to > >>> a future version > >>> > >>> -Jordan > >>> > >>> On May 24, 2020, at 3:58 PM, Enrico Olivelli <eolive...@gmail.com> > >>> > >>> wrote: > >>> > >>> > >>> > >>> > >>> Il Dom 24 Mag 2020, 22:48 Cameron McKenzie <cammcken...@apache.org > >>> > >>> <mailto:cammcken...@apache.org <mailto:cammcken...@apache.org> < > cammcken...@apache.org <mailto:cammcken...@apache.org>>>> ha scritto: > >>> > >>> Enrico, > >>> Can you explain your environment that exposes these backwards > >>> compatibility issues? > >>> > >>> Cameron, > >>> Let's say we have two libraries Foo and Bar that are compiled for > >>> > >>> Curator 4.x. > >>> > >>> > >>> I am now using in my Application Baz that use both Foo and Bar. So I > >>> > >>> have Curator 4.x on the classpath. > >>> > >>> Developers of Foo want to move to Curator 5.x in Foo 2.0, but Bar is > >>> > >>> still happy with Curator 4.x. > >>> > >>> > >>> If I want to upgrade Foo to 2.0 I have these chances: > >>> 1) Curator 5 is compatible with 4.x,so I can simply keep 5 and > >>> > >>> everything works > >>> > >>> 2) Curator 5 is not compatible with 4.x so I can't have both (this is > >>> > >>> current case) > >>> > >>> 3) Curator 5 is independent from 4.x and I can keep both of them > >>> > >>> The best option for users is 1). > >>> > >>> 3) is good anyway, but it needs more work for users that want to > migrate. > >>> > >>> Option 2) is not good. Users will have to shade/relocate Curator 5 or 4 > >>> > >>> and Foo 2.0 or Bar. > >>> > >>> > >>> Hope that this explains better the problem > >>> Enrico > >>> > >>> > >>> I am probably coming from a place of ignorance, but I > >>> haven't seen new versions of a third party binary being dropped into an > >>> existing environment without recompiling the application, so I have > never > >>> encountered these binary compatibility issues before. My expectation > with > >>> this release was that if you wanted to pickup the changes in Curator > 5.0 > >>> that you would rebuild your application against the new binaries and > then > >>> redeploy the application. Obviously this compilation will break if you > >>> > >>> are > >>> > >>> using any of the changed APIs, but they are pretty trivial change to > fix. > >>> We could potentially deprecate the existing APIs and add the new ones, > >>> > >>> but > >>> > >>> this will produce more tech debt to clean up later. > >>> cheers > >>> > >>> On Sat, May 23, 2020 at 7:40 PM Enrico Olivelli <eolive...@gmail.com > <mailto:eolive...@gmail.com> > >>> > >>> <mailto:eolive...@gmail.com <mailto:eolive...@gmail.com> < > eolive...@gmail.com <mailto:eolive...@gmail.com>>>> wrote: > >>> > >>> > >>> I will check you trick ad soon as possible. I am sorry, this is a very > >>> busy week for me and do not have enough cycles. But I think that we > >>> > >>> should > >>> > >>> address this problem in order to ease the adoption of the new code and > >>> > >>> APIs. > >>> > >>> > >>> Did you evaluate to eventually rollback the breaking changes? > >>> > >>> Another alternative, if we want to let users use both the old and the > >>> > >>> new > >>> > >>> APIs is to simply rename all of the packages and start a brand new > >>> > >>> system. > >>> > >>> This approach was done in Apache Commons and IIRC it will be done with > >>> Netty5. We also did it with the new Apache Bookkeeper API. > >>> > >>> Pros: > >>> No need to preserve compatibility, we are free to clean up all of the > >>> > >>> tech > >>> > >>> debt. > >>> The switch to Curator 5 will be explicit opted in > >>> > >>> Cons: > >>> Cherry picks won't be straightforward. > >>> > >>> Enrico > >>> > >>> Il Ven 22 Mag 2020, 23:40 Jordan Zimmerman <jor...@jordanzimmerman.com > <mailto:jor...@jordanzimmerman.com> > >>> > >>> <mailto:jor...@jordanzimmerman.com <mailto:jor...@jordanzimmerman.com> > <jor...@jordanzimmerman.com <mailto:jor...@jordanzimmerman.com>>>> > >>> > >>> ha scritto: > >>> > >>> Hi Everyone, > >>> > >>> I've coded a possible solution in the test project. See here: > >>> > >>> > >>> > https://github.com/Randgalt/curator_5_0_test/blob/master/combo/pom.xml#L49 > < > https://github.com/Randgalt/curator_5_0_test/blob/master/combo/pom.xml#L49 > > > >>> < > >>> > https://github.com/Randgalt/curator_5_0_test/blob/master/combo/pom.xml#L49 > < > https://github.com/Randgalt/curator_5_0_test/blob/master/combo/pom.xml#L49 > > > >>> > >>> > >>> > >>> It uses the Maven dependency plugin to create a small compatibility > >>> > >>> JAR > >>> > >>> that contains the Curator 4.3.0 versions of the classes that have > >>> > >>> changed > >>> > >>> in 5.0.0 (i.e. the ones that no longer return ListenerContainer). If > >>> > >>> this > >>> > >>> JAR is included in a CLASSPATH before Curator 5.0.0's JARs, these old > >>> classes will take precedence and thus old binaries will continue to > >>> > >>> work. > >>> > >>> The curator_5_0_test shows this. run.sh is the previous way with the > >>> error. run-compatibility.sh is with the compatibility JAR. > >>> > >>> Thoughts? Notable, this doesn't change the master code of Curator at > >>> > >>> all. > >>> > >>> We could add it to the 5.0 release. I don't think there's an issue > >>> > >>> with > >>> > >>> this "hack". Can anyone think of one? I'd really appreciate people > >>> > >>> testing > >>> > >>> with it. Try a build with just Curator 5.0 and then install and > >>> > >>> include > >>> > >>> this curator-5_0-test:combo:1.0-SNAPSHOT early in the CLASSPATH - it > >>> > >>> should > >>> > >>> work. > >>> > >>> -Jordan > >>> > >>> On May 21, 2020, at 10:43 AM, Jordan Zimmerman < > >>> jor...@jordanzimmerman.com <mailto:jor...@jordanzimmerman.com> > <mailto:jor...@jordanzimmerman.com <mailto:jor...@jordanzimmerman.com> > >>> <jor...@jordanzimmerman.com>>> > >>> > >>> wrote: > >>> > >>> > >>> Hello All, > >>> > >>> Sorry for the cross-posting but this is important enough to justify > >>> > >>> it. > >>> > >>> > >>> Apache Curator is in the process of releasing version 5.0. We've taken > >>> the opportunity to address some long standing tech debt but this > >>> > >>> causes > >>> > >>> breaking changes. We've detailed the breaks here: > >>> http://curator.apache.org/staging/breaking-changes.html < > >>> > >>> http://curator.apache.org/staging/breaking-changes.html>. The Clirr > >>> > >>> report shows the exact API changes: > >>> http://curator.apache.org/staging/curator-recipes/clirr-report.html < > >>> > >>> http://curator.apache.org/staging/curator-recipes/clirr-report.html>. > The > >>> > >>> first two of these are the most worrisome. NodeCache's and > >>> PathChildrenCache's getListenable() methods now have a different > >>> > >>> return > >>> > >>> type. This has far reaching implications. If a Curator user were to > >>> > >>> drop in > >>> > >>> Curator 5.0 without any code changes they will get runtime exceptions > >>> > >>> when > >>> > >>> these methods are called. > >>> > >>> I've written a test that shows the problem: > >>> > >>> git clone https://github.com/Randgalt/curator_5_0_test.git < > >>> > >>> https://github.com/Randgalt/curator_5_0_test.git> > >>> > >>> cd curator_5_0_test > >>> ./run.sh > >>> > >>> You will see: > >>> > >>> java.lang.NoSuchMethodError: > >>> > >>> > >>> > org.apache.curator.framework.recipes.cache.PathChildrenCache.getListenable()Lorg/apache/curator/framework/listen/ListenerContainer; > >>> > >>> at binary.Curator50Test.run(Curator50Test.java:26) > >>> at test.Test.main(Test.java:9) > >>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > >>> at > >>> > >>> > >>> > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > >>> > >>> at > >>> > >>> > >>> > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > >>> > >>> at java.lang.reflect.Method.invoke(Method.java:498) > >>> at org.codehaus.mojo.exec.ExecJavaMojo$1.run(ExecJavaMojo.java:297) > >>> at java.lang.Thread.run(Thread.java:748) > >>> > >>> Enrico Olivelli brought this to our attention. Curator 5.0 is a major > >>> version bump so breaking changes are implied. But, maybe this is > >>> > >>> blocker? > >>> > >>> What do people think? If this is a serious enough concern we can come > >>> > >>> up > >>> > >>> with a workaround. > >>> > >>> Please discuss and let's hold off completing the current release until > >>> this has been fully discussed. > >>> > >>> -Jordan > >
