OK, based on your confirmation Enrico and no other feedback from the community, I will move onto releasing this in the next few days. cheers
On Thu, May 28, 2020 at 6:30 AM Enrico Olivelli <[email protected]> wrote: > Il Mer 27 Mag 2020, 17:38 Jordan Zimmerman <[email protected]> ha > scritto: > > > BookKeeper is not using the two recipes then, NodeCache and > > PathChildrenCache. I think we can safely release and document the > > workaround for those that need it. > > > > Just to be clear. > > I have cited Bookkeeper but my tests where mostly about a few other non OS > projects in my company. > > Bookkeeper uses Curator only on the server side, and actually the server > code usually is rebuilt at every release. > > The problems I described can arise when Curator is used on clients, like > for Service Discovery or metadata management. > > > Enrico > > > > -Jordan > > > > On May 27, 2020, at 9:51 AM, Enrico Olivelli <[email protected]> > wrote: > > > > 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 < > > [email protected]> 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 <[email protected]> > > > > 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 <[email protected] > > > > <mailto:[email protected] <[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]> < > > mailto:[email protected] <[email protected]>> < > > > > [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]>> > > > > > > <mailto:[email protected] <[email protected]> < > > mailto:[email protected] <[email protected]>> < > > > > [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]>> > > > > > > <mailto:[email protected] <[email protected]> < > > mailto:[email protected] <[email protected]>> > > > > <[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 > > > > > > < > > > > > 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]> > > > > <mailto:[email protected] <[email protected]> < > > mailto:[email protected] <[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 > > > > > > >
