Both pull requests are now merged in master and branch-3.5. Please let me know if you spot any issues.
I've also reopened ZOOKEEPER-2635 <https://issues.apache.org/jira/browse/ZOOKEEPER-2635> as a reminder to regenerate documents for 3.5.3 release. On Sat, Feb 4, 2017 at 10:43 PM, Michael Han <h...@cloudera.com> wrote: > We now have two pull requests pending for this issue per discussion in > https://github.com/apache/zookeeper/pull/122 > > The PR for master branch is now: #152 > <https://github.com/apache/zookeeper/pull/152> > The PR for branch-3.5 is now: #151 > <https://github.com/apache/zookeeper/pull/151> > > I've tested both pull requests locally and everything looks good. Please > have a look if anyone has any cycles next week - I plan to merge both by > end of next week (2/10) so we can get 3.5.3 release moving forward. > > On Thu, Jan 5, 2017 at 12:17 PM, Michael Han <h...@cloudera.com> wrote: > >> Patch looking good for ZOOKEEPER-2642 (https://github.com/apache/zoo >> keeper/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. >> > > > > -- > Cheers > Michael. > -- Cheers Michael.
