> On July 10, 2014, 5:50 p.m., Sean Busbey wrote:
> > minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java,
> >  line 259
> > <https://reviews.apache.org/r/23397/diff/1/?file=627812#file627812line259>
> >
> >     nit: whitesapce

Are these nits for the patch or nits about whitespace changes? Because for 
whatever reason this file is not properly formatted and if we're stripping all 
whitespace changes from patches, I don't know when we expect these to be 
addressed. If we're going to nit everyone over whitespace issues outside the 
scope of the patch, we're going to discourage people from auto formatting files 
they're editing, which could result in even worse formatting nits being 
introduced in their code.


> On July 10, 2014, 5:50 p.m., Sean Busbey wrote:
> > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java,
> >  lines 267-268
> > <https://reviews.apache.org/r/23397/diff/1/?file=627813#file627813line267>
> >
> >     nit: can we skip this change in line wrapping?
> >     
> >     I think this change set is already likely to conflict with 
> > ACCUMULO-2944 and I'd like to minimize it.

Addressing the two 2944 conflict items with r2


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23397/#review47585
-----------------------------------------------------------


On July 10, 2014, 5:24 p.m., John Vines wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23397/
> -----------------------------------------------------------
> 
> (Updated July 10, 2014, 5:24 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2984
>     https://issues.apache.org/jira/browse/ACCUMULO-2984
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Adds a change to SiteConfiguration to allow external setting of the xml 
> configuration file.
> Adds a single method to MiniAccumuloConfig which allows a user to point to 
> accumulo-site.xml and HADOOP_CONF_DIR to use for pulling out requisite 
> instance information
> Clusters configurations into those required to run inside a MAC-sized 
> footprint and those which are for arbitrary naming schemes for MAC
> Provides flagging to prevent uneccessary folder creation
> Provides flagging to prevent running zookeeper and initializing
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java 
> 4c7d95e 
>   
> minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java
>  be80f85 
>   
> minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
>  977968e 
>   
> minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java
>  337eda0 
> 
> Diff: https://reviews.apache.org/r/23397/diff/
> 
> 
> Testing
> -------
> 
> Ran the following test code-
> public class TestMACWithRealInstance {
>   public static void main(String args[]) throws IOException, 
> AccumuloException, AccumuloSecurityException, TableExistsException, 
> InterruptedException {
>     MiniAccumuloConfig macConfig = new MiniAccumuloConfig(new 
> File("/tmp/mac"), "secret");
>     macConfig.setNumTservers(2);
>     macConfig.setMemory(ServerType.TABLET_SERVER, 2, MemoryUnit.GIGABYTE);
>     macConfig.useExistingInstance(new 
> File("/usr/lib/accumulo/conf/accumulo-site.xml"), new 
> File("/usr/lib/hadoop/conf"));
>     MiniAccumuloCluster mac = new MiniAccumuloCluster(macConfig);
>     mac.start();
>     System.out.println("Started");
>     mac.getConnector("root", "secret").tableOperations().create("macCreated");
>     System.out.println("Stopping");
>     mac.stop();
>     System.out.println("Stopped");
>   }
> }
> Which runs fine, except stopping issues which seem to be related to 
> ACCUMULO-2985
> 
> After running this, I validated that the table was created in the real 
> accumulo instance via zkCli
> 
> 
> Thanks,
> 
> John Vines
> 
>

Reply via email to