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]>> 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]>> 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]>> > > > 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]>> > 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 > > >> > > >> > > >> > >
