> 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 <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 <mailto: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> >> < >> 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>>> 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>>> 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>>> >>>> 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>>> >> 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> < >> 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> < >> 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> < >> 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
