On Wed, Dec 6, 2017 at 11:28 AM, Josh Elser <els...@apache.org> wrote: > 1.a sounds better to me.
why? > > A would be the ideal solution, I think B is the next best if A doesn't work. > > I need to get the Hadoop3 compatibility fixed, so I'll be investigating the > Hadoop shaded artifacts this week. > > > On 12/5/17 6:43 PM, Christopher wrote: >> >> I was wondering about Hadoop 3 shading and whether that would help us. It >> would be really nice if it could, or if there was some other class path >> solution that was easy. >> >> I think there are two major issues in this thread. The first is the API >> problems. The second is the Hadoop 3 support. They are related, but I >> think >> quickly dealing with the API issues can clarify what our options are for >> Hadoop 3. >> >> >> >> >> To fix the API, I would like to get consensus on proceeding with this >> path: >> >> 1. Rename 1.8.2-SNAPSHOT to 1.9.0-SNAPSHOT and deprecate the existing >> ZooKeeperInstance constructor which takes a Configuration >> a) Deprecate ClientConfiguration and replace with ClientConfig (or a >> better name) which does not extend Configuration or have API leak >> problems, >> and add a new ZKI constructor for this >> b) Ignore extends for now, and drop it from ClientConfiguration in >> 2.0 >> with a break (can't deprecate superclass), and add new ZKI constructor for >> more specific ClientConfiguration next to deprecated one >> 2. Drop deprecated stuff from 2.0 branch (and extends, if option 1.b was >> chosen) >> 3. Plan a 1.9.0 release instead of 1.8.2 >> >> I prefer 1.a over 1.b, personally, but I've been tossing back and forth. I >> would need input on which is best. There are pros and cons to both, >> regarding churn, and source and binary compatibility. >> >> >> >> >> Once we deal with the API, our options for Hadoop 3 become: >> >> A. Use Hadoop 3 shaded artifacts or some other class path solution (such >> as >> getting lucky identifying a version of commons-beanutils that works for >> both) >> B. Shade in 1.9 with a breaking change >> C. Create a 1.9 version named 2.0, so we can do a breaking change without >> semver violation; shade in this version >> D. Shade in the branch we're currently calling 2.0 >> >> I think we can defer that decision pending some further >> investigation/experimentation into what works, and deal with it after >> dealing with steps 1-3 above (but soon after, hopefully). >> >> >> >> On Tue, Dec 5, 2017 at 3:58 PM Josh Elser <els...@apache.org> wrote: >> >>> Another potential suggestion I forgot about: we try to just move to the >>> Hadoop shaded artifacts. This would invalidate the need to do more, but >>> I have no idea how "battle-tested" those artifacts are. >>> >>> On 12/5/17 3:52 PM, Keith Turner wrote: >>>> >>>> If we do the following. >>>> >>>> * Drop ZooKeeperInstance.ZooKeeperInstance(Configuration config) >>> >>> method. >>>> >>>> * Drop extends from ClientConfig >>>> * Add a method ZooKeeperInstance.ZooKeeperInstance(ClientConfig >>>> config) >>>> >>>> Then this will not be binary compatible, so it will still be painful >>>> in many cases. It may be source compatible. >>>> >>>> For example the following will be source (but not binary) compatible. >>>> >>>> ClientConfiguration cc = new ClientConfiguration(file); >>>> //when compiled against older version of Accumulo will bind to >>>> method with commons config signature >>>> //when recompiled will bind to clientconfig version of method >>>> ZooKeeperInstance zki = new ZooKeeperInstance(cc); >>>> >>>> The following would not be source or binary compatible. >>>> >>>> Configuration cc = new ClientConfiguration(file); >>>> ZooKeeperInstance zki = new ZooKeeperInstance(cc); >>>> >>>> >>>> On Tue, Dec 5, 2017 at 3:40 PM, Josh Elser <els...@apache.org> wrote: >>>>> >>>>> >>>>> >>>>> On 12/5/17 3:28 PM, Keith Turner wrote: >>>>>> >>>>>> >>>>>> On Tue, Dec 5, 2017 at 2:53 PM, Josh Elser<els...@apache.org> wrote: >>>>>>> >>>>>>> >>>>>>> Interesting. What makes you want to deprecate ClientConfig entirely? >>>>>>> >>>>>>> I'd be worried about removing without sufficient thought of >>> >>> replacement >>>>>>> >>>>>>> around. It would be a bit "churn-y" to introduce yet another way that >>>>>>> clients have to connect (since it was introduced in 1.6-ish?). >>>>>>> Working >>>>>>> around the ClientConfig changes was irritating for the downstream >>>>>>> integrations (Hive, most notably). >>>>>> >>>>>> >>>>>> Ok maybe thats a bad idea, not looking to cause pain. Here were some >>>>>> of my goals. >>>>>> >>>>>> * Remove commons config from API completely via deprecation cycle. >>>>>> * Introduce API that supports putting all props needed to connect >>>>>> to >>>>>> Accumulo in an API. >>>>>> >>>>>> I suppose if we want to keep ClientConfig class in API, then there is >>>>>> no way to remove commons config via a deprecation cycle?? We can't >>>>>> deprecate the extension of commons config, all we can do is just drop >>>>>> it at some point. >>>>>> >>>>> >>>>> My line of thinking is that the majority of the time, we're creating a >>>>> ClientConfiguration by one of: >>>>> >>>>> * ClientConfiguration#loadDefault() >>>>> * new ClientConfiguration(String) >>>>> * new ClientConfiguration(File) >>>>> >>>>> Granted, we also inherit/expose a few other things (notably extending >>>>> CompositeConfiguration and throwing ConfigurationException). I would be >>>>> comfortable with dropping those w/o deprecation. I have not seen >>> >>> evidence >>>>> >>>>> from anyone that they are widely in use by folks (although I've not >>>>> explicitly asked, either). >>> >>> >> >