accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mwa...@apache.org
Subject [accumulo] 03/04: Code review updates for PR #623
Date Fri, 31 Aug 2018 15:05:43 GMT
This is an automated email from the ASF dual-hosted git repository.

mwalch pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit ec35002420aaf7e2fa7dedaa3ff53468d5ee9fec
Author: Mike Walch <mwalch@apache.org>
AuthorDate: Thu Aug 30 13:29:53 2018 -0400

    Code review updates for PR #623
    
    * Renamed accumulo.configuration property to accumulo.properties
---
 .../accumulo/core/conf/SiteConfiguration.java      |  4 +-
 .../minicluster/impl/MiniAccumuloClusterImpl.java  |  2 +-
 .../minicluster/impl/MiniAccumuloConfigImpl.java   |  2 +-
 server/base/pom.xml                                |  9 ++--
 .../start/classloader/AccumuloClassLoader.java     |  2 +-
 .../accumulo/test/RewriteTabletDirectoriesIT.java  | 15 ++----
 .../java/org/apache/accumulo/test/VolumeIT.java    | 56 ++++++++--------------
 .../apache/accumulo/test/start/KeywordStartIT.java |  2 +
 8 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
index 9512bd2..f9553c9 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java
@@ -44,7 +44,7 @@ import com.google.common.base.Preconditions;
  * the -o option) and then from accumulo.properties. This implementation supports defaulting
  * undefined property values to a parent configuration's definitions.
  * <p>
- * The system property "accumulo.configuration" can be used to specify the location of the
+ * The system property "accumulo.properties" can be used to specify the location of the
  * properties file on the classpath or filesystem if the path is prefixed with 'file://'.
If the
  * system property is not defined, it defaults to "accumulo.properties" and will look on
classpath
  * for file.
@@ -126,7 +126,7 @@ public class SiteConfiguration extends AccumuloConfiguration {
           + "classpath. Since 2.0.0, this file was replaced by 'accumulo.properties'.");
     }
 
-    String configFile = System.getProperty("accumulo.configuration", "accumulo.properties");
+    String configFile = System.getProperty("accumulo.properties", "accumulo.properties");
     if (configFile.startsWith("file://")) {
       try {
         File f = new File(new URI(configFile));
diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
index 0d2fb08..ea12809 100644
--- a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
+++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
@@ -356,7 +356,7 @@ public class MiniAccumuloClusterImpl implements AccumuloCluster {
       confMap.putAll(config.getSiteConfig());
       confMap.putAll(configOverrides);
       writeConfigProperties(siteFile, confMap);
-      jvmOpts.add("-Daccumulo.configuration=" + siteFile.getName());
+      jvmOpts.add("-Daccumulo.properties=" + siteFile.getName());
     }
 
     if (config.isJDWPEnabled()) {
diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java
b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java
index 7c0e3b7..5dbd663 100644
--- a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java
+++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java
@@ -712,7 +712,7 @@ public class MiniAccumuloConfigImpl {
 
     this.existingInstance = Boolean.TRUE;
 
-    System.setProperty("accumulo.configuration", "accumulo.properties");
+    System.setProperty("accumulo.properties", "accumulo.properties");
     this.hadoopConfDir = hadoopConfDir;
     hadoopConf = new Configuration(false);
     accumuloConf = new SiteConfiguration(accumuloProps);
diff --git a/server/base/pom.xml b/server/base/pom.xml
index c6cfea2..96523a3 100644
--- a/server/base/pom.xml
+++ b/server/base/pom.xml
@@ -101,10 +101,6 @@
       <artifactId>slf4j-api</artifactId>
     </dependency>
     <dependency>
-      <groupId>org.slf4j</groupId>
-      <artifactId>slf4j-log4j12</artifactId>
-    </dependency>
-    <dependency>
       <groupId>org.apache.hadoop</groupId>
       <artifactId>hadoop-client-runtime</artifactId>
       <scope>runtime</scope>
@@ -119,6 +115,11 @@
       <artifactId>easymock</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-log4j12</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <build>
     <testResources>
diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/AccumuloClassLoader.java
b/start/src/main/java/org/apache/accumulo/start/classloader/AccumuloClassLoader.java
index ae33f58..ee56f8d 100644
--- a/start/src/main/java/org/apache/accumulo/start/classloader/AccumuloClassLoader.java
+++ b/start/src/main/java/org/apache/accumulo/start/classloader/AccumuloClassLoader.java
@@ -41,7 +41,7 @@ public class AccumuloClassLoader {
   private static final Logger log = LoggerFactory.getLogger(AccumuloClassLoader.class);
 
   static {
-    String configFile = System.getProperty("accumulo.configuration", "accumulo.properties");
+    String configFile = System.getProperty("accumulo.properties", "accumulo.properties");
     if (configFile.startsWith("file://")) {
       try {
         File f = new File(new URI(configFile));
diff --git a/test/src/main/java/org/apache/accumulo/test/RewriteTabletDirectoriesIT.java b/test/src/main/java/org/apache/accumulo/test/RewriteTabletDirectoriesIT.java
index 42a1189..3b4fd55 100644
--- a/test/src/main/java/org/apache/accumulo/test/RewriteTabletDirectoriesIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/RewriteTabletDirectoriesIT.java
@@ -21,9 +21,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
-import java.io.BufferedOutputStream;
 import java.io.File;
-import java.io.FileOutputStream;
 import java.util.Collections;
 import java.util.Map.Entry;
 import java.util.SortedSet;
@@ -46,6 +44,7 @@ import org.apache.accumulo.server.init.Initialize;
 import org.apache.accumulo.server.util.Admin;
 import org.apache.accumulo.server.util.RandomizeVolumes;
 import org.apache.accumulo.test.functional.ConfigurableMacBase;
+import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.RawLocalFileSystem;
@@ -130,14 +129,10 @@ public class RewriteTabletDirectoriesIT extends ConfigurableMacBase
{
       cluster.stop();
 
       // add the 2nd volume
-      Configuration conf = new Configuration(false);
-      conf.addResource(
-          new Path(cluster.getConfig().getConfDir().toURI().toString(), "accumulo.properties"));
-      conf.set(Property.INSTANCE_VOLUMES.getKey(), v1 + "," + v2);
-      BufferedOutputStream fos = new BufferedOutputStream(
-          new FileOutputStream(new File(cluster.getConfig().getConfDir(), "accumulo.properties")));
-      conf.writeXml(fos);
-      fos.close();
+      PropertiesConfiguration conf = new PropertiesConfiguration();
+      conf.load(cluster.getAccumuloPropertiesPath());
+      conf.setProperty(Property.INSTANCE_VOLUMES.getKey(), v1 + "," + v2);
+      conf.save(cluster.getAccumuloPropertiesPath());
 
       // initialize volume
       assertEquals(0, cluster.exec(Initialize.class, "--add-volumes").waitFor());
diff --git a/test/src/main/java/org/apache/accumulo/test/VolumeIT.java b/test/src/main/java/org/apache/accumulo/test/VolumeIT.java
index 70619a0..5c6ae34 100644
--- a/test/src/main/java/org/apache/accumulo/test/VolumeIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/VolumeIT.java
@@ -20,9 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
-import java.io.BufferedOutputStream;
 import java.io.File;
-import java.io.FileOutputStream;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -71,6 +69,7 @@ import org.apache.accumulo.server.log.WalStateManager.WalMarkerException;
 import org.apache.accumulo.server.log.WalStateManager.WalState;
 import org.apache.accumulo.server.util.Admin;
 import org.apache.accumulo.test.functional.ConfigurableMacBase;
+import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
@@ -285,19 +284,15 @@ public class VolumeIT extends ConfigurableMacBase {
     Assert.assertEquals(0, cluster.exec(Admin.class, "stopAll").waitFor());
     cluster.stop();
 
-    Configuration conf = new Configuration(false);
-    conf.addResource(
-        new Path(cluster.getConfig().getConfDir().toURI().toString(), "accumulo.properties"));
+    PropertiesConfiguration conf = new PropertiesConfiguration();
+    conf.load(cluster.getAccumuloPropertiesPath());
 
     File v3f = new File(volDirBase, "v3");
     assertTrue(v3f.mkdir() || v3f.isDirectory());
     Path v3 = new Path("file://" + v3f.getAbsolutePath());
 
-    conf.set(Property.INSTANCE_VOLUMES.getKey(), v1 + "," + v2 + "," + v3);
-    BufferedOutputStream fos = new BufferedOutputStream(
-        new FileOutputStream(new File(cluster.getConfig().getConfDir(), "accumulo.properties")));
-    conf.writeXml(fos);
-    fos.close();
+    conf.setProperty(Property.INSTANCE_VOLUMES.getKey(), v1 + "," + v2 + "," + v3);
+    conf.save(cluster.getAccumuloPropertiesPath());
 
     // initialize volume
     Assert.assertEquals(0, cluster.exec(Initialize.class, "--add-volumes").waitFor());
@@ -330,19 +325,15 @@ public class VolumeIT extends ConfigurableMacBase {
     Assert.assertEquals(0, cluster.exec(Admin.class, "stopAll").waitFor());
     cluster.stop();
 
-    Configuration conf = new Configuration(false);
-    conf.addResource(
-        new Path(cluster.getConfig().getConfDir().toURI().toString(), "accumulo.properties"));
+    PropertiesConfiguration conf = new PropertiesConfiguration();
+    conf.load(cluster.getAccumuloPropertiesPath());
 
     File v3f = new File(volDirBase, "v3");
     assertTrue(v3f.mkdir() || v3f.isDirectory());
     Path v3 = new Path("file://" + v3f.getAbsolutePath());
 
-    conf.set(Property.INSTANCE_VOLUMES.getKey(), v2 + "," + v3);
-    BufferedOutputStream fos = new BufferedOutputStream(
-        new FileOutputStream(new File(cluster.getConfig().getConfDir(), "accumulo.properties")));
-    conf.writeXml(fos);
-    fos.close();
+    conf.setProperty(Property.INSTANCE_VOLUMES.getKey(), v2 + "," + v3);
+    conf.save(cluster.getAccumuloPropertiesPath());
 
     // initialize volume
     Assert.assertEquals(0, cluster.exec(Initialize.class, "--add-volumes").waitFor());
@@ -494,15 +485,10 @@ public class VolumeIT extends ConfigurableMacBase {
     Assert.assertEquals(0, cluster.exec(Admin.class, "stopAll").waitFor());
     cluster.stop();
 
-    Configuration conf = new Configuration(false);
-    conf.addResource(
-        new Path(cluster.getConfig().getConfDir().toURI().toString(), "accumulo.properties"));
-
-    conf.set(Property.INSTANCE_VOLUMES.getKey(), v2.toString());
-    BufferedOutputStream fos = new BufferedOutputStream(
-        new FileOutputStream(new File(cluster.getConfig().getConfDir(), "accumulo.properties")));
-    conf.writeXml(fos);
-    fos.close();
+    PropertiesConfiguration conf = new PropertiesConfiguration();
+    conf.load(cluster.getAccumuloPropertiesPath());
+    conf.setProperty(Property.INSTANCE_VOLUMES.getKey(), v2.toString());
+    conf.save(cluster.getAccumuloPropertiesPath());
 
     // start cluster and verify that volume was decommissioned
     cluster.start();
@@ -552,16 +538,12 @@ public class VolumeIT extends ConfigurableMacBase {
     Assert.assertTrue("Failed to rename " + v2f + " to " + v9f, v2f.renameTo(v9f));
     Path v9 = new Path(v9f.toURI());
 
-    Configuration conf = new Configuration(false);
-    conf.addResource(
-        new Path(cluster.getConfig().getConfDir().toURI().toString(), "accumulo.properties"));
-
-    conf.set(Property.INSTANCE_VOLUMES.getKey(), v8 + "," + v9);
-    conf.set(Property.INSTANCE_VOLUMES_REPLACEMENTS.getKey(), v1 + " " + v8 + "," + v2 +
" " + v9);
-    BufferedOutputStream fos = new BufferedOutputStream(
-        new FileOutputStream(new File(cluster.getConfig().getConfDir(), "accumulo.properties")));
-    conf.writeXml(fos);
-    fos.close();
+    PropertiesConfiguration conf = new PropertiesConfiguration();
+    conf.load(cluster.getAccumuloPropertiesPath());
+    conf.setProperty(Property.INSTANCE_VOLUMES.getKey(), v8 + "," + v9);
+    conf.setProperty(Property.INSTANCE_VOLUMES_REPLACEMENTS.getKey(),
+        v1 + " " + v8 + "," + v2 + " " + v9);
+    conf.save(cluster.getAccumuloPropertiesPath());
 
     // start cluster and verify that volumes were replaced
     cluster.start();
diff --git a/test/src/main/java/org/apache/accumulo/test/start/KeywordStartIT.java b/test/src/main/java/org/apache/accumulo/test/start/KeywordStartIT.java
index 928ba7f..2659311 100644
--- a/test/src/main/java/org/apache/accumulo/test/start/KeywordStartIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/start/KeywordStartIT.java
@@ -50,6 +50,7 @@ import org.apache.accumulo.proxy.Proxy;
 import org.apache.accumulo.server.conf.ConfigSanityCheck;
 import org.apache.accumulo.server.init.Initialize;
 import org.apache.accumulo.server.util.Admin;
+import org.apache.accumulo.server.util.ConvertConfig;
 import org.apache.accumulo.server.util.Info;
 import org.apache.accumulo.server.util.LoginProperties;
 import org.apache.accumulo.server.util.ZooKeeperMain;
@@ -104,6 +105,7 @@ public class KeywordStartIT {
     expectSet.put("admin", Admin.class);
     expectSet.put("check-server-config", ConfigSanityCheck.class);
     expectSet.put("classpath", Classpath.class);
+    expectSet.put("convert-config", ConvertConfig.class);
     expectSet.put("create-token", CreateToken.class);
     expectSet.put("gc", GCExecutable.class);
     expectSet.put("help", Help.class);


Mime
View raw message