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.
