On Wed, Mar 26, 2014 at 11:38 AM, Josh Elser <[email protected]> wrote:
> > > On 3/26/14, 12:10 AM, Sean Busbey wrote: > >> ACCUMULO-2143 has developed a conversation about MiniAccumuloCluster's >> intended use and the way we currently implement the difference between MAC >> for external use and MAC for internal Accumulo testing[1]. >> >> In particular, Josh had a few major concerns >> >> ----- >> >> It doesn't make sense to me why MiniAccumuloCluster is a concrete class >> which, ultimately still tied to a MiniAccumuloClusterImpl. >> MiniAccumuloCluster *requires* a MiniAccumuloClusterImpl or something that >> extends it. This is what's really chafing me about the separation of >> "accumulo user" and "accumulo developer" methods - you *always* have them >> both. Not to mention, this hierarchy is really obnoxious to create a new >> implementation of AccumuloMiniCluster(Impl) because I have to carry all of >> the cruft of the "original" implementation with me. >> >> Bringing this back around to this ticket, while I still don't agree with >> the reasoning that exposing the FileSystem or ZooKeeper object that >> MiniAccumuloClusterImpl is getting us anything other than the ability to >> say "we didn't change this [arbitrary] API". For "users" who might not >> care >> what the underlying FileSystem or ZooKeeper connection, it's merely an >> extra two items in their editor's code-completion. For "users" who would >> care to use this information, we now make them jump through extra hoops to >> get it. That just doesn't make any sense to me for something we haven't >> even released. >> >> To be honest, I really want to re-open >> ACCUMULO-2151<https://issues.apache.org/jira/browse/ACCUMULO-2151>, >> make MiniAccumuloCluster an interface, MiniAccumuloClusterImpl an >> implementation of said interface, and create some factory class to make >> instances, ala Connector.tableOperations, Connector.securityOperations, >> etc. Right now there's a class we call an "API" that cannot be generically >> extended for the sake of saying "we have an API". >> >> ---- >> >> I wanted to avoid having a drawn out discussion on a jira, where folks my >> not notice it. Especially with things being late in 1.6.0 development and >> the potential this has to impact the API. >> >> Personally, I don't have much of a dog in the fight. There's always some >> arbitrary line for where the public API will be, presuming we want to have >> any kind of balance between providing a stable based for others to build >> on >> and being able to refactor things. I would like us to hold to our API >> promises[2] and I would rather we not leak implementation details >> unnecessarily. >> >> I suspect the choice to make MiniAccumuloCluster a class rather than an >> interface with a factory was driven by the restrictions we put on API >> changes between major versions and the fact that 1.5 had a class you could >> instantiate via constructors[3]. >> > > Ok, that makes the most sense to me - I hadn't previously considered the > "deprecation" cycle since it previously wasn't held to that standard. I > mocked up some changes to better match the rest of our "public API" objects > (class, interfaces, factories). If we want to preserve this API > compatibility from earlier versions, we *could* name the interfaces classes > (what would normally be MiniAccumuloCluster and MiniAccumuloConfig) to > something non-standard to support API compatibility (just requiring a > re-compilation I think). > > I would be behind getting the interfaces how we want them long term before > categorizing these classes as "public". I'm willing to make sure we're all > happy with this for 1.6.0. There is a slow way to introduce an interface. 1) Depreciate MAC construnctors and add factory in 1.7.0 2) In 1.10.0 drop constructor and change to interface. > > > It's possible we can address some of the major reusability concerns by >> moving most of the implementation back into MAC, liberally using package >> access for members, and making the internal-use MAC extend the public one. >> >> >> [1]: https://issues.apache.org/jira/browse/ACCUMULO-2143 >> [2]: http://accumulo.apache.org/governance/releasing.html >> [3]: https://issues.apache.org/jira/browse/ACCUMULO-2151 >> >>
