accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bil...@apache.org
Subject accumulo git commit: ACCUMULO-3742 add ClientConfiguration constructor that reads from a file and handles multi-valued properties correctly
Date Tue, 21 Apr 2015 17:16:44 GMT
Repository: accumulo
Updated Branches:
  refs/heads/1.6 f0b3487bb -> f5c7f05af


ACCUMULO-3742 add ClientConfiguration constructor that reads from a file and handles multi-valued
properties correctly


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/f5c7f05a
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/f5c7f05a
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/f5c7f05a

Branch: refs/heads/1.6
Commit: f5c7f05af7bdb95a2485bc0566bd2c4968f1427f
Parents: f0b3487
Author: Billie Rinaldi <billie@apache.org>
Authored: Tue Apr 21 10:12:23 2015 -0700
Committer: Billie Rinaldi <billie@apache.org>
Committed: Tue Apr 21 10:12:23 2015 -0700

----------------------------------------------------------------------
 .../apache/accumulo/core/cli/ClientOpts.java    |  3 +-
 .../core/client/ClientConfiguration.java        | 32 +++++++++++++++++++-
 .../core/util/shell/ShellOptionsJC.java         |  4 +--
 .../core/conf/ClientConfigurationTest.java      | 21 +++++++++++++
 .../src/test/resources/multi-valued.client.conf | 16 ++++++++++
 .../minicluster/MiniAccumuloInstance.java       |  5 ++-
 6 files changed, 74 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
index 8bb8b3f..4d00d97 100644
--- a/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
+++ b/core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java
@@ -46,7 +46,6 @@ import org.apache.accumulo.core.security.ColumnVisibility;
 import org.apache.accumulo.core.volume.VolumeConfiguration;
 import org.apache.accumulo.core.zookeeper.ZooUtil;
 import org.apache.accumulo.trace.instrument.Trace;
-import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.mapreduce.Job;
@@ -230,7 +229,7 @@ public class ClientOpts extends Help {
       if (clientConfigFile == null)
         clientConfig = ClientConfiguration.loadDefault();
       else
-        clientConfig = new ClientConfiguration(new PropertiesConfiguration(clientConfigFile));
+        clientConfig = new ClientConfiguration(clientConfigFile);
     } catch (Exception e) {
       throw new IllegalArgumentException(e);
     }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java b/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java
index 17ad10b..91cda6a 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/ClientConfiguration.java
@@ -27,10 +27,13 @@ import java.util.UUID;
 import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.core.conf.PropertyType;
 import org.apache.accumulo.core.util.ArgumentChecker;
+import org.apache.commons.configuration.AbstractConfiguration;
 import org.apache.commons.configuration.CompositeConfiguration;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.PropertiesConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Contains a list of property keys recognized by the Accumulo client and convenience methods
for setting them.
@@ -38,6 +41,8 @@ import org.apache.commons.configuration.PropertiesConfiguration;
  * @since 1.6.0
  */
 public class ClientConfiguration extends CompositeConfiguration {
+  private static final Logger log = LoggerFactory.getLogger(ClientConfiguration.class);
+
   public static final String USER_ACCUMULO_DIR_NAME = ".accumulo";
   public static final String USER_CONF_FILENAME = "config";
   public static final String GLOBAL_CONF_FILENAME = "client.conf";
@@ -105,10 +110,31 @@ public class ClientConfiguration extends CompositeConfiguration {
     }
   };
 
+  public ClientConfiguration(String configFile) throws ConfigurationException {
+    this(new PropertiesConfiguration(), configFile);
+  }
+
+  private ClientConfiguration(PropertiesConfiguration propertiesConfiguration, String configFile)
throws ConfigurationException {
+    super(propertiesConfiguration);
+    // Don't do list interpolation
+    propertiesConfiguration.setListDelimiter('\0');
+    propertiesConfiguration.load(configFile);
+  }
+
   public ClientConfiguration(List<? extends Configuration> configs) {
     super(configs);
     // Don't do list interpolation
     this.setListDelimiter('\0');
+    for (Configuration c : configs) {
+      if (c instanceof AbstractConfiguration) {
+        AbstractConfiguration abstractConfiguration = (AbstractConfiguration) c;
+        if (abstractConfiguration.getListDelimiter() != '\0') {
+          log.warn("Client configuration constructed with a Configuration that did not have
list delimiter overridden, multi-valued config properties may " +
+              "be unavailable");
+          abstractConfiguration.setListDelimiter('\0');
+        }
+      }
+    }
   }
 
   /**
@@ -143,7 +169,10 @@ public class ClientConfiguration extends CompositeConfiguration {
       for (String path : paths) {
         File conf = new File(path);
         if (conf.canRead()) {
-          configs.add(new PropertiesConfiguration(conf));
+          PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration();
+          propertiesConfiguration.setListDelimiter('\0');
+          propertiesConfiguration.load(conf);
+          configs.add(propertiesConfiguration);
         }
       }
       return new ClientConfiguration(configs);
@@ -154,6 +183,7 @@ public class ClientConfiguration extends CompositeConfiguration {
 
   public static ClientConfiguration deserialize(String serializedConfig) {
     PropertiesConfiguration propConfig = new PropertiesConfiguration();
+    propConfig.setListDelimiter('\0');
     try {
       propConfig.load(new StringReader(serializedConfig));
     } catch (ConfigurationException e) {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
index dfa24c5..9206c8d 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
@@ -28,7 +28,6 @@ import org.apache.accumulo.core.client.ClientConfiguration;
 import org.apache.accumulo.core.client.ClientConfiguration.ClientProperty;
 import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
 import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.log4j.Logger;
 
 import com.beust.jcommander.DynamicParameter;
@@ -270,8 +269,7 @@ public class ShellOptionsJC {
   }
 
   public ClientConfiguration getClientConfiguration() throws ConfigurationException, FileNotFoundException
{
-    ClientConfiguration clientConfig = clientConfigFile == null ? ClientConfiguration.loadDefault()
: new ClientConfiguration(new PropertiesConfiguration(
-        getClientConfigFile()));
+    ClientConfiguration clientConfig = clientConfigFile == null ? ClientConfiguration.loadDefault()
: new ClientConfiguration(getClientConfigFile());
     if (useSsl()) {
       clientConfig.setProperty(ClientProperty.INSTANCE_RPC_SSL_ENABLED, "true");
     }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java
index 40be70f..0ecc519 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java
@@ -17,12 +17,14 @@
 package org.apache.accumulo.core.conf;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
 import java.util.Arrays;
 
 import org.apache.accumulo.core.client.ClientConfiguration;
 import org.apache.accumulo.core.client.ClientConfiguration.ClientProperty;
 import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.PropertiesConfiguration;
 import org.junit.Test;
 
@@ -63,4 +65,23 @@ public class ClientConfigurationTest {
     third.addProperty(ClientProperty.INSTANCE_ZK_TIMEOUT.getKey(), "123s");
     return new ClientConfiguration(Arrays.asList(first, second, third));
   }
+
+  @Test
+  public void testMultipleValues() throws ConfigurationException {
+    String val = "comma,separated,list";
+    PropertiesConfiguration propertiesConfiguration = new PropertiesConfiguration();
+    propertiesConfiguration.setListDelimiter('\0');
+    propertiesConfiguration.addProperty(ClientProperty.INSTANCE_ZK_HOST.getKey(), val);
+    ClientConfiguration conf = new ClientConfiguration(propertiesConfiguration);
+    assertEquals(val, conf.get(ClientProperty.INSTANCE_ZK_HOST));
+    assertEquals(1, conf.getList(ClientProperty.INSTANCE_ZK_HOST.getKey()).size());
+
+    conf = new ClientConfiguration(new PropertiesConfiguration("multi-valued.client.conf"));
+    assertNotEquals(val, conf.get(ClientProperty.INSTANCE_ZK_HOST));
+    assertEquals(3, conf.getList(ClientProperty.INSTANCE_ZK_HOST.getKey()).size());
+
+    conf = new ClientConfiguration("multi-valued.client.conf");
+    assertEquals(val, conf.get(ClientProperty.INSTANCE_ZK_HOST));
+    assertEquals(1, conf.getList(ClientProperty.INSTANCE_ZK_HOST.getKey()).size());
+  }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/core/src/test/resources/multi-valued.client.conf
----------------------------------------------------------------------
diff --git a/core/src/test/resources/multi-valued.client.conf b/core/src/test/resources/multi-valued.client.conf
new file mode 100644
index 0000000..457370b
--- /dev/null
+++ b/core/src/test/resources/multi-valued.client.conf
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+instance.zookeeper.host=comma,separated,list

http://git-wip-us.apache.org/repos/asf/accumulo/blob/f5c7f05a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
----------------------------------------------------------------------
diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
b/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
index fb6fb0a..cc6c2ed 100644
--- a/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
+++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
@@ -43,7 +43,10 @@ public class MiniAccumuloInstance extends ZooKeeperInstance {
 
   public static PropertiesConfiguration getConfigProperties(File directory) {
     try {
-      return new PropertiesConfiguration(new File(new File(directory, "conf"), "client.conf"));
+      PropertiesConfiguration conf = new PropertiesConfiguration();
+      conf.setListDelimiter('\0');
+      conf.load(new File(new File(directory, "conf"), "client.conf"));
+      return conf;
     } catch (ConfigurationException e) {
       // this should never happen since we wrote the config file ourselves
       throw new IllegalArgumentException(e);


Mime
View raw message