Patch looking good for ZOOKEEPER-2642 ( https://github.com/apache/zookeeper/pull/122).
I just tested locally with both stress unit tests and zkCli.sh, everything looks fine. If anyone has any cycles could you please review / test this patch? I'd like to have more +1s before getting this in - and it would be good if we can get this in sooner so we can cut out a release candidate build for 3.5.3 (beta). On Thu, Dec 8, 2016 at 2:50 PM, Jordan Zimmerman <jor...@jordanzimmerman.com > wrote: > Thanks for the replies everyone. Adding the methods back with a > deprecation notice solves Curator’s problems so I’ll proceed with that on > ZOOKEEPER-2642. > > -Jordan > > > On Dec 8, 2016, at 6:56 PM, Michael Han <h...@cloudera.com> wrote: > > > > Thanks Jordan for raising the concern. It's a reasonable concern with > good > > intention to make user's life easier, which is important to the project. > > > > There are two separate issues we need to reach a consensus here: > > > > * Move reconfig API > > The details of why the decision was made is documented in ZOOKEEPER-2014 > > JIRA, but just to recap, the API is moved because it's the right thing to > > do in long term, which yield a cleaner interface design for ZooKeeper > > client API. In future, we might also want to clean up other APIs (i.e. > move > > quota related APIs over as well.) as well. While seemingly stylish as it > > might look like, I believe a clean API is also important for a project's > > usability / adoption. From the discussions so far I think everyone either > > agree with this, or have no strong opinion against it, so I consider this > > question is solved. > > > > * Backward compatibility between releases > > Since we decide refactor reconfig API, the question is when. The reconfig > > API was removed from ZOOKEEPER-2014 (which targets 3.5.3 alpha) because > of > > the definition of alpha / beta releases in the context of ZooKeeper > project > > (I'm not saying this is right or wrong, simply state a fact up to date): > an > > alpha release implies that we allow backward incompatible changes, and a > > beta release implies that the APIs are locked down and none backward > > compatible change. Given this definition, it is perfectly legitimate to > > move API around w/o considering compatibility. Now what Jordan pointed > out > > regarding definition of 'alpha' combined with some old discussion > threads I > > found in user list indicate that our definition of 'alpha', and 'beta' is > > none standard and could cause confusions. So I think we should try reach > a > > consensus on whether we stick with current definition of 'alpha' and > 'beta' > > release during 3.5.3 release cycle, or adopt something else. > > > > With all these said, I think options are: > > * Do nothing, stick with current alpha / beta definition. > > * Try redefine what 'alpha' and 'beta' means in the context of ZooKeeper > > and then go from there in 3.5.3 release cycle. > > > > In any cases, I'd like to propose we keep the reconfig API in > > ZooKeeperAdmin which as previously stated is the right thing to do in > long > > term. If we decide to maintain backward API compatibility for 3.5.3-beta > > release, we could add reconfig API back to ZooKeeper but mark it as > > deprecated, then remove the API in next major release (3.6 or 4.x). > > > > > > On Thu, Dec 8, 2016 at 8:27 AM, Patrick Hunt <ph...@apache.org> wrote: > > > >> I've only seen a few questions about versioning come up on the ZK lists, > >> they were quickly responded to with pointers to the process. iirc we > based > >> our versioning scheme on what Hadoop was/is using. Additionally if we > don't > >> allow for alpha time it will further slow progress as there will be no > >> opportunity to adjust things like APIs once they are committed and > >> generally available for people to "kick the tires". > >> > >> I'll leave it up to the rest of the community to state their opinions, > but > >> as i have stated imo it would be a mistake to revert this change. Jordan > >> has raised a reasonable concern - I consider this a blocker for > 3.5.3-alpha > >> until it is resolved. > >> > >> Patrick > >> > >> On Thu, Dec 8, 2016 at 1:46 AM, Jordan Zimmerman < > >> jor...@jordanzimmerman.com > >>> wrote: > >> > >>>> I think people understand what alpha means. > >>> > >>> With respect, the ZooKeeper team has used “alpha” in a novel way that > is > >>> sowing considerable confusion. I wrote emails about this a while back. > >> But, > >>> here again, is another case where the non-standard usage of “alpha” > will > >>> cause confusion. To reiterate: someone who sees "3.5.2-alpha” will > think > >>> that there will eventually be a “3.5.2-beta” and finally a “3.5.2” > >> release > >>> version. But, with ZooKeeper there will never be a “3.5.2” release. In > >>> fact, the “-alpha” label is mis-located. The real version is > >>> “3.5.?-alpha1”, “3.5.?-alpha2”, etc. There’s an important implication > >> here. > >>> If a ZooKeeper user who doesn’t follow the mailing lists, etc. sees > >>> “3.5.2-alpha” and “3.5.3-alpha” they will mentally see these as two > >>> different releases. What ZOOKEEPER-2014 has done is remove an existing > >> API > >>> from what appears to be a released version 3.5.2 (which never really > >>> existed). This change violates semantic versioning. For external users, > >> the > >>> version after “3.5.2” should be “4.x.x” as it has breaking changes. > >>> > >>>> It's not about style, there were a number of concerns addressed in > that > >>>> patch. > >>> > >>> The auth issues are very real ones. The issues in ZOOKEEPER-2014 can be > >>> addressed completely without moving the reconfig() methods to a new > >> class. > >>> It may be useful to move APIs around for clarity, etc. but these > breaking > >>> changes should be for a version that signals breaking changes - 4.x.x > or > >>> something. i.e. moving the reconfig() APIs is orthogonal to the > concerns > >> of > >>> ZOOKEEPER-2014 and should be a separate Jira issue. > >>> > >>>> I think people understand what alpha means. There may be some short > >> term > >>>> impact for a few, but a significant benefit over the long term. > >>> > >>> Again with respect - 3.5.0-alpha was made available in Maven Central > >>> August 2014 - over 2 years ago. Regardless of the ZooKeeper team’s > >> intent, > >>> this is NOT an alpha in users’ minds. This is a released library that > is > >> in > >>> production in major organizations. The ZooKeeper team should realize > this > >>> and adjust their thinking about the “alpha” tag. For Apache Curator, > >> we’re > >>> now put in a position where the reconfig() APIs have disappeared. In > >> order > >>> to maintain our own internal semantic versioning we will have to > >> consider a > >>> third branch of Curator (we currently have a ZooKeeper 3.4.x compatible > >>> version and a ZooKeeper 3.5.x compatible version). It would be awesome > if > >>> we didn’t have to do this. > >>> > >>> -Jordan > >>> > >>>> On Dec 7, 2016, at 7:16 PM, Patrick Hunt <ph...@apache.org> wrote: > >>>> > >>>> It's not about style, there were a number of concerns addressed in > that > >>>> patch. We didn't take the change lightly, we've been discussing it > over > >>>> jira and the mailing list over the past two years. > >>>> > >>>> I think people understand what alpha means. There may be some short > >> term > >>>> impact for a few, but a significant benefit over the long term. > >>>> > >>>> Patrick > >>>> > >>>> On Dec 7, 2016 9:24 AM, "Jordan Zimmerman" < > jor...@jordanzimmerman.com > >>> > >>>> wrote: > >>>> > >>>>> I read through the issue and disagree about the decision to move the > >>> APIs > >>>>> out. That was a stylistic choice that breaks client code. I realize > >> that > >>>>> 3.5.x has been advertised as an alpha but you must realize that most > >> of > >>> the > >>>>> world is using it in production. These APIs have now been published. > >>> This > >>>>> will create a real headache for Curator which is why I’ve created > >>>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2642 < > >>>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2642> - I hope we > can > >>>>> move these APIs back into ZooKeeper.java. > >>>>> > >>>>> -Jordan > >>>>> > >>>>>> On Dec 7, 2016, at 5:54 PM, Patrick Hunt <ph...@apache.org> wrote: > >>>>>> > >>>>>> It's discussed in more depth in the JIRA iirc, but basically; > >>>>>> ZooKeeper.java provides client APIs, reconfig is an admiistrative > >> API. > >>>>>> > >>>>>> Patrick > >>>>>> > >>>>>> On Wed, Dec 7, 2016 at 8:48 AM, Jordan Zimmerman < > >>>>> jor...@jordanzimmerman.com > >>>>>>> wrote: > >>>>>> > >>>>>>> I understand the need to make the methods require proper auth but > >>>>> there's > >>>>>>> no reason to move it to a different class that I can see. Am I > >> missing > >>>>>>> something? > >>>>>>> > >>>>>>> ==================== > >>>>>>> Jordan Zimmerman > >>>>>>> > >>>>>>>> On Dec 7, 2016, at 4:37 PM, Patrick Hunt <ph...@apache.org> > wrote: > >>>>>>>> > >>>>>>>> This problem has been a long standing blocker issue for 3.5 and > >>>>>>> identified > >>>>>>>> early on as something that would need to change. This has been one > >> of > >>>>> the > >>>>>>>> reasons why 3.5 has stayed in alpha - because we allow > non-backward > >>>>>>>> compatible changes to new APIs in alpha and we knew we would have > >> to > >>>>> fix > >>>>>>>> this. The description/comments of ZOOKEEPER-2014 does a good job > >>>>>>>> documenting why this had to change. > >>>>>>>> > >>>>>>>> Patrick > >>>>>>>> > >>>>>>>> On Wed, Dec 7, 2016 at 3:18 AM, Jordan Zimmerman < > >>>>>>> jor...@jordanzimmerman.com > >>>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> OK - I found the offending issue: ZOOKEEPER-2014 > >>>>>>>>> > >>>>>>>>> What is the benefit/logic of moving the reconfig() variants into > a > >>> new > >>>>>>>>> class? I can see if this was done from the start but you have now > >>>>> broken > >>>>>>>>> Curator in a fairly serious non-backward compatible way for a > >> minor > >>>>>>>>> documenting benefit. Does anyone object to me reversing this? > >>>>>>>>> > >>>>>>>>> -Jordan > >>>>>>>>> > >>>>>>>>>> On Dec 7, 2016, at 11:37 AM, Jordan Zimmerman < > >>>>>>>>> jor...@jordanzimmerman.com> wrote: > >>>>>>>>>> > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> I was compiling Curator against the ZK master and noticed that > >> the > >>>>>>>>> reconfig APIs are gone/changed. Can anyone point me at the issues > >>> for > >>>>>>> this > >>>>>>>>> and/or the discussion why this breaking change was made? > >>>>>>>>>> > >>>>>>>>>> -Jordan > >>>>>>>>> > >>>>>>>>> > >>>>>>> > >>>>> > >>>>> > >>> > >>> > >> > > > > > > > > -- > > Cheers > > Michael. > > -- Cheers Michael.
