On Wed, Dec 6, 2017 at 3:37 PM Keith Turner <ke...@deenlo.com> wrote:
> On Wed, Dec 6, 2017 at 2:28 PM, Christopher <ctubb...@apache.org> wrote: > > On Wed, Dec 6, 2017 at 2:10 PM Josh Elser <els...@apache.org> wrote: > > > >> > >> > >> On 12/6/17 2:06 PM, Christopher wrote: > >> > On Wed, Dec 6, 2017 at 1:55 PM Keith Turner <ke...@deenlo.com> wrote: > >> > > >> >> On Wed, Dec 6, 2017 at 1:43 PM, Josh Elser <els...@apache.org> > wrote: > >> >>> > >> >>> > >> >>> On 12/6/17 12:17 PM, Keith Turner wrote: > >> >>>> > >> >>>> On Wed, Dec 6, 2017 at 11:56 AM, Josh Elser<els...@apache.org> > >> wrote: > >> >>>>> > >> >>>>> Maybe a difference in interpretation: > >> >>>>> > >> >>>>> I was seeing 1a as being source-compatible still. My assumption > was > >> >> that > >> >>>>> "Deprecate ClientConfiguration" meant that it would remain in the > >> >>>>> codebase > >> >>>>> -- "replace" as in "replace expected user invocation", not > removal of > >> >> the > >> >>>>> old ClientConfiguration and addition of a new ClientConfig class. > >> >>>> > >> >>>> Ok, if we deprecate ClientConfiguration, leave it in 2.0, and drop > the > >> >>>> extends from ClientConfiguration in 2.0. Then I am not sure what > the > >> >>>> benefit of introducing the new ClientConfig type is? > >> >>> > >> >>> > >> >>> I read this as leaving the extends in ClientConfiguration and > dropping > >> >> that > >> >>> in the new ClientConfig. Agree, I wouldn't see the point in changing > >> the > >> >>> parent class of ClientConfiguration (as that would break things). > >> >> > >> >> > >> >> I don't think we can leave ClientConfiguration as deprecated and > >> >> extending commons config in Accumulo 2.0. This leaves commons config > >> >> 1 in the API. > >> >> > >> >> Personally I am not in favor of dropping ClientConfiguration in 2.0, > >> >> which is why I was in favor option b. > >> >> > >> > > >> > In the absence of any further input from others, I'll follow along > with > >> > whatever you and Josh can agree on. Although I lean towards option > 1.a, I > >> > don't feel strongly about either option. We can also do a vote if > neither > >> > of you is able (or willing) to convince the other of your preference. > >> > >> I don't feel strongly enough either way to raise a stink. Color me > >> surprised that Keith is the one to encourage quick removals from API :) > >> > >> If he's OK with it, I'm fine with it. I was trying to err on the side of > >> less breakage. > >> > > > > Sorry to rehash this, but your wording about Keith encouraging quick > > removals confused me, so I want to make sure we're on the same page. > > > > As I understand it, Keith favors the option (1.b) to remove the > superclass > > from ClientConfiguration in 2.0, rather than remove the whole class. This > > results in no source breakage as long as nobody used inherited methods > from > > the superclass or assigned (or cast, implicitly or explicitly) it to the > > superclass type. It does have binary breakage in 2.0. So, I don't think > > Keith is advocating for quick removals, but the opposite, which has > > *complicated* compatibility expectations but possibly less breakage under > > specific conditions. > > This email helped me see a subtle difference I did not see. Its about > whether code compiled against 1.9 (w/ not deprecation warnings) will > work in 2.0. > > For option 1.a > > * IF user compiles against 1.9 AND resolves all deprecation warnings then > their > code is guaranteed to work with 2.0. > * Can not have source compat for code that used ClientConfiguration when > going > from 1.6,1.7,1.8 to 2.0 and skipping step above. > > For option 1.b > > * IF user compiles against 1.9 AND resolves all deprecation warnings then > their > code MAY work with 2.0. Using inherited commons config methods goes > undetected. > * May have source compat for code that used ClientConfiguration when > going from > 1.6,1.7,1.8 to 2.0. > > What prevents source compat for 1.6,1.7,1.8 to 2.0 for option 1.a is > dropping ClientConfiguration type in 2.0 > > Another option to consider is 1.c where the ClientConfiguration type > is not dropped in 2.0. The following would be done for 1.c > > * Deprecate ZooKeeperInstance constuctor that takes commons config type > in 1.9 > * Deprecate ClientConfiguration type in 1.9 > * Add ClientConfig type in 1.9 > * Add ZooKeeperInstance constructor that takes ClientConfig type in 1.9 > * Add ZooKeeperInstance constructor that takes ClientConfiguration type > in 1.9 > * Drop ZooKeeperInstance constuctor that takes commons config type in 2.0 > * Drop extends commons config type from ClientConfiguration in 2.0. > * Do NOT drop ClientConfiguration type in 2.0 > > For option 1.c > > * IF user compiles against 1.9 AND resolves all deprecation warnings then > their > code is guaranteed to work with 2.0. > * May have source compat for code that used ClientConfiguration when > going from > 1.6,1.7,1.8 to 2.0. > > > > > The other option (1.a) would result in no source or binary breakage, > except > > if the deprecation warnings are ignored, in which case, there would be a > > source and binary breakage in 2.0. This option may also create churn if > we > > make further improve the entry point to Accumulo in 2.0 or later. The > > potential for breakage is a bit more predictable and well-defined in this > > option. > If I understand your option 1.c correctly, it's basically the same as option 1.b, but with some elements of 1.a so that it gains the property of being able to smoothly transition from 1.9 to 2.0 if deprecations are respected. I like this property of 1.a, but I think this adds too much complexity, and I don't like the idea of introducing a deprecated method from the start, nor the idea of leaving 2.0 with two classes which basically are identical, neither of which is deprecated. I'm fine with just doing option 1.b. In spite of the confusion I expressed in my last post to this thread, Josh did seem willing to go with that option, too. So, I'll start a PR to start working on that.
