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