Re: reconfig APIs

2017-02-11 Thread Michael Han
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 as a reminder to regenerate documents for 3.5.3 release. On Sat, Feb 4, 2017 at 10:43 PM, Michael Han

Re: reconfig APIs

2017-02-04 Thread Michael Han
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 The PR for branch-3.5 is now: #151 I've tested

Re: reconfig APIs

2017-01-05 Thread Michael Han
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

Re: reconfig APIs

2016-12-09 Thread Arshad Mohammad
+1 for having this feature only in master branch(3.6.0) +1 for reverting the change from branch-3.5 On Fri, Dec 9, 2016 at 4:20 AM, Jordan Zimmerman wrote: > Thanks for the replies everyone. Adding the methods back with a > deprecation notice solves Curator’s

Re: reconfig APIs

2016-12-08 Thread Jordan Zimmerman
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 wrote: > > Thanks Jordan for raising the concern. It's a reasonable

Re: reconfig APIs

2016-12-08 Thread Michael Han
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

Re: reconfig APIs

2016-12-08 Thread Patrick Hunt
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

Re: reconfig APIs

2016-12-08 Thread Jordan Zimmerman
> 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

Re: reconfig APIs

2016-12-07 Thread Patrick Hunt
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

Re: reconfig APIs

2016-12-07 Thread Jordan Zimmerman
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.

Re: reconfig APIs

2016-12-07 Thread Patrick Hunt
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 wrote: > I understand the need to make the methods require proper auth but

Re: reconfig APIs

2016-12-07 Thread Jordan Zimmerman
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 wrote: > > This problem has been a

Re: reconfig APIs

2016-12-07 Thread Patrick Hunt
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.

Re: reconfig APIs

2016-12-07 Thread Jordan Zimmerman
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