You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by mw...@apache.org on 2018/08/31 15:05:43 UTC
[accumulo] 03/04: Code review updates for PR #623
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 <mw...@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);