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). >
I would also be comfortable with that. I think the main API that we need to deal with is the ZooKeeperInstance(Configuration) constructor. It should be ZooKeeperInstance(ClientConfiguration) instead. If we assume that the only way this method was used was with ClientConfiguration objects, then simply changing the parameter type will continue to be source compatible. I don't know if we want to assume that. Either way, it won't be binary compatible. If we're not concerned with binary compatibility, and are willing to make that assumption about this constructor only (or usually) being used with ClientConfiguration, then we can just change the method signature now, without deprecation cycles or a new alternative method.
