Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Received-SPF: softfail (nike.apache.org: transitioning domain of
 wilhelm.von.cloud@accumulo.net does not designate 209.85.160.169 as permitted
 sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAGUtCHpyMQBguxfVJ2_So4qtao5wGQ8YX23RL3U4fMVWKsty2g@mail.gmail.com>
References: 
 <CAGHyZ6LCofhLaChRgifEJ9XxajVQLxbgnrOTvSmZf_-WqvcPNQ@mail.gmail.com>
	<CAGUtCHpyMQBguxfVJ2_So4qtao5wGQ8YX23RL3U4fMVWKsty2g@mail.gmail.com>
Date: Wed, 26 Mar 2014 13:10:57 -0400
Message-ID: 
 <CAMz+DusHDBgux3mZScMLOCuVgsQZvFQNYWY2qnOmWU_zHo9Cuw@mail.gmail.com>
Subject: Re: [DISCUSS] MiniAccumuloCluster goals and approach
From: William Slacum <wilhelm.von.cloud@accumulo.net>
To: dev@accumulo.apache.org
Content-Type: multipart/alternative; boundary=20cf303a320b55ccee04f5858dfd

--20cf303a320b55ccee04f5858dfd
Content-Type: text/plain; charset=ISO-8859-1

[NOTE: I started this email when this thread was new, and it kinda of blew
up on me while writing it and being distracted. Apologies in advance if
things were already covered or it's not relevant any more.]

Is this a design quality discussion or a a functionality discussion?

The changes from 1.5->1.6 seem like a poor design decision, but they do aid
in functionality.

>From 1.5:
  public MiniAccumuloCluster(File dir, String rootPassword) throws
IOException
  public MiniAccumuloCluster(MiniAccumuloConfig config) throws IOException
  public void start() throws IOException, InterruptedException
  public String getInstanceName()
  public String getZooKeepers()
  public void stop() throws IOException, InterruptedException

>From 1.6:
  public MiniAccumuloCluster(File dir, String rootPassword) throws
IOException
  public MiniAccumuloCluster(MiniAccumuloConfig config) throws IOException
  public void start() throws IOException, InterruptedException
  public Set<Pair<ServerType,Integer>> getDebugPorts()
  public String getInstanceName()
  public String getZooKeepers()
  public void stop() throws IOException, InterruptedException
  public MiniAccumuloConfig getConfig()
  public Connector getConnector(String user, String passwd) throws
AccumuloException, AccumuloSecurityException
  public ClientConfiguration getClientConfig()

>From a client perspective, I see a difference of #getDebugPorts,
#getConfig, #getConnector, #getClientConfig. The other methods are on the
Impl. There's nothing wrong with using aggregation in this case, since the
code would be the same regardless.

I don't quite understand what it means to "extend generically." At this
point, the MiniAccumuloCluster's interface of the MiniAccumuloClusterImpl's
interface. The naming could, and should, be better, but I don't quite get
where we're losing functionality.


On Wed, Mar 26, 2014 at 12:06 PM, Keith Turner <keith@deenlo.com> 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.
>
> 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.
>
>
>
>
> On Wed, Mar 26, 2014 at 3:10 AM, Sean Busbey <busbey+lists@cloudera.com
> >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
> >
>

--20cf303a320b55ccee04f5858dfd--