#298 - More review updates

apache · Nov 10, 2014 · 03e35f5 · 03e35f5
1 parent 2a56925
commit 03e35f5
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 23 deletions.
diff --git a/modules/api/src/main/java/io/fluo/api/config/FluoConfiguration.java b/modules/api/src/main/java/io/fluo/api/config/FluoConfiguration.java
@@ -479,6 +479,14 @@ private boolean contains(String key) {
     return true;
   }
 
+  private boolean unsetStringProperty(String key) {
+    if (containsKey(key) && !getString(key).isEmpty()) {
+      log.info(key + " should not be set");
+      return false;
+    }
+    return true;
+  }
+
   /**
    * Returns true if required properties for FluoClient are set
    */
@@ -523,12 +531,22 @@ public boolean hasRequiredWorkerProps() {
    */
   public boolean hasRequiredMiniFluoProps() {
     boolean valid = true;
-    if (getMiniStartAccumulo() == false) {
+    if (getMiniStartAccumulo()) {
+      // insure client properties are not set since we are using MiniAccumulo
+      valid &= unsetStringProperty(CLIENT_ACCUMULO_USER_PROP);
+      valid &= unsetStringProperty(CLIENT_ACCUMULO_PASSWORD_PROP);
+      valid &= unsetStringProperty(CLIENT_ACCUMULO_INSTANCE_PROP);
+      valid &= unsetStringProperty(CLIENT_ACCUMULO_ZOOKEEPERS_PROP);
+      valid &= unsetStringProperty(CLIENT_ZOOKEEPER_CONNECT_PROP);
+      if (valid == false) {
+        log.error("Client properties should not be set in your configuration if MiniFluo is configured to start its own accumulo (indicated by io.fluo.mini.start.accumulo being set to true)");
+      }
+    } else {
       valid &= hasRequiredClientProps();
-    }
-    valid &= hasRequiredAdminProps();
-    valid &= hasRequiredOracleProps();
-    valid &= hasRequiredWorkerProps();
+      valid &= hasRequiredAdminProps();
+      valid &= hasRequiredOracleProps();
+      valid &= hasRequiredWorkerProps();
+    } 
     return valid;
   }
 

diff --git a/modules/api/src/test/java/io/fluo/api/config/FluoConfigurationTest.java b/modules/api/src/test/java/io/fluo/api/config/FluoConfigurationTest.java
@@ -150,8 +150,11 @@ public void testHasOracleProps() {
   @Test
   public void testHasMiniFluoProps() {
     FluoConfiguration config = new FluoConfiguration();
-    Assert.assertFalse(config.hasRequiredMiniFluoProps());
+    Assert.assertTrue(config.hasRequiredMiniFluoProps());
     config.setAccumuloUser("user");
+    Assert.assertFalse(config.hasRequiredMiniFluoProps());
+    config.setMiniStartAccumulo(false);
+    Assert.assertFalse(config.hasRequiredMiniFluoProps());
     config.setAccumuloPassword("pass");
     config.setAccumuloInstance("instance");
     config.setAccumuloTable("table");

diff --git a/modules/cluster/src/main/java/io/fluo/cluster/mini/MiniAdmin.java b/modules/cluster/src/main/java/io/fluo/cluster/mini/MiniAdmin.java
@@ -52,11 +52,7 @@ public static void main(String[] args) {
 
     try {
       FluoConfiguration config = new FluoConfiguration(new File(options.getFluoProps()));
-      if (!config.hasRequiredMiniFluoProps()) {
-        log.error("fluo.properties is missing required properties for MiniFluo");
-        System.exit(-1);
-      }
-
+
       switch (options.getCommand().toLowerCase()) {
         case "stop":
           File dataDir = new File(config.getMiniDataDir());

diff --git a/modules/cluster/src/main/java/io/fluo/cluster/mini/MiniFluoMain.java b/modules/cluster/src/main/java/io/fluo/cluster/mini/MiniFluoMain.java
@@ -23,7 +23,6 @@
 import io.fluo.api.mini.MiniFluo;
 import io.fluo.cluster.util.Log4jUtil;
 import io.fluo.cluster.util.MainOptions;
-import org.apache.commons.configuration.PropertiesConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -60,14 +59,7 @@ public static void main(String[] args) {
       }
       try (MiniFluo mini = FluoFactory.newMiniFluo(config)) {
         log.info("MiniFluo is running");
-
-        File miniProps = new File(config.getMiniDataDir()+"/client.properties");
-        PropertiesConfiguration connConfig = new PropertiesConfiguration();
-        connConfig.append(mini.getClientConfiguration());
-        connConfig.save(miniProps);
-
-        log.info("Wrote MiniFluo client properties to {}", miniProps.getAbsolutePath());
-
+
         while (true) {
           Thread.sleep(100);
         }

diff --git a/modules/core/src/main/java/io/fluo/core/mini/MiniFluoImpl.java b/modules/core/src/main/java/io/fluo/core/mini/MiniFluoImpl.java
@@ -35,6 +35,7 @@
 import org.apache.accumulo.minicluster.MiniAccumuloCluster;
 import org.apache.accumulo.minicluster.MiniAccumuloConfig;
 import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -86,7 +87,7 @@ private synchronized boolean isProcessing(Scanner scanner) {
 
   public MiniFluoImpl(FluoConfiguration fluoConfig) {
     if (!fluoConfig.hasRequiredMiniFluoProps()) {
-      throw new IllegalArgumentException("MiniFluo configuration is missing required properties");
+      throw new IllegalArgumentException("MiniFluo configuration is not valid");
     }
     config = fluoConfig;
 
@@ -110,7 +111,7 @@ public MiniFluoImpl(FluoConfiguration fluoConfig) {
       throw new RuntimeException(e);
     }
   }
-
+  
   private void startMiniAccumulo() {
     try {
       // start mini accumulo cluster
@@ -135,7 +136,14 @@ private void startMiniAccumulo() {
 
       FluoFactory.newAdmin(config).initialize();
 
-      log.info("Initialized Fluo for MiniAccumulo");
+      log.info("Initialized MiniFluo");
+
+      File miniProps = new File(config.getMiniDataDir()+"/client.properties");
+      PropertiesConfiguration connConfig = new PropertiesConfiguration();
+      connConfig.append(config.getClientConfiguration());
+      connConfig.save(miniProps);
+
+      log.info("Wrote MiniFluo client properties to {}", miniProps.getAbsolutePath());
 
     } catch (Exception e) {
       throw new RuntimeException(e);