> 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 > > John Vines wrote: > 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. > > Josh Elser wrote: > The eclipse formatter, at least how I have it set up with the > configuration we have set, will be adding these extra spaces.
The nits are about the whitespace being left in the changes the patch introduces. Generally I only push back on formatting errors related to the scope of the patch. Yes, there is a bug in the eclipse formatter that causes it to incorrectly miss the end of line whitespace in javadoc sections it populates. - Sean ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23397/#review47585 ----------------------------------------------------------- On July 10, 2014, 6:16 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, 6:16 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 > >
