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 <[email protected]> wrote: > > > Il giorno mar 26 mag 2020 alle ore 21:53 Jordan Zimmerman < > [email protected]> 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 <[email protected]> >> 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 <[email protected]> wrote: >> >> Il giorno dom 24 mag 2020 alle ore 23:17 Jordan Zimmerman < >> [email protected]> 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 <[email protected]> >> >> wrote: >> >> >> >> >> Il Dom 24 Mag 2020, 22:48 Cameron McKenzie <[email protected] >> >> <mailto:[email protected] <[email protected]>>> 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 <[email protected] >> >> <mailto:[email protected] <[email protected]>>> 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 <[email protected] >> >> <mailto:[email protected] <[email protected]>>> >> >> 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 >> >> >> >> 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 < >> [email protected] <mailto:[email protected] >> <[email protected]>>> >> >> 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 >> >> >> >>
