You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by bi...@apache.org on 2015/04/21 19:16:44 UTC
accumulo git commit: ACCUMULO-3742 add ClientConfiguration
constructor that reads from a file and handles multi-valued properties
correctly
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 <bi...@apache.org>
Authored: Tue Apr 21 10:12:23 2015 -0700
Committer: Billie Rinaldi <bi...@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);