On Wed, Mar 26, 2014 at 12:12 PM, Josh Elser <[email protected]> wrote:
> On 3/26/14, 9:06 AM, Keith Turner wrote: > >> There were many change made to MAC so Accumulo could test itself. For >> example a method was added to return the internal threads that flush logs. >> Flushing the logs may be a useful feature to add. However it could be >> offered in a way that does not expose these internal threads. When >> working on ACCUMULO-2151 I had no desire to reimplement things like this, >> I just wanted to hide it. It was hidden from users so we do not have to >> support it and can change it at will when testing 1.7.0. >> > > That's my irk with it. The changes we made "hide" things for no other > purpose than saying "we hid them". The next variant of a MAC is going to > have to re-architect the entire thing anyways (I'm doing this right now and > I'm overhauling it). > There is a purpose. Whats an alternative solution to the addition of "public List<LogWriter> getLogWriters()" to the MAC API? If you want to re-write MAC all you have to support is the interface in minicluster, you are free to throw everything in minicluster.impl away. > > It doesn't make sense to me ship something that changes it without > addressing the underlying problems. I don't want to suggest it because I > don't want to introduce our own mapred and mapreduce dichotomy, but I can't > come up with a better alternative yet. > > > As Sean said MAC was a class in 1.4.4, 1.5.0, and 1.5.1. So making it an >> interface would break things for any users using it. Any reorganizing of >> the implementation of MAC could easily be done after 1.6.0. From a users >> perspective the MAC API has changed very little, even though the >> implementation has dramatically changed. >> > > Reorganizing of *this single implementation of MAC* can easily be done. > Any extension or reimplementation will be dirty, and inherit a large bit of > code that is likely useless. > > > >> On Wed, Mar 26, 2014 at 3:10 AM, Sean Busbey <[email protected]> >> 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]. >>> >>> 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 >>> >>> >>
