ACCUMULO-4600: Fix to properly read from accumulo-site.xml
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/80373557 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/80373557 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/80373557 Branch: refs/heads/master Commit: 80373557b4d39c10b3dd62dca5d13d7ed0290b5d Parents: de80cf5 Author: Mike Miller <mmil...@apache.org> Authored: Wed Mar 8 16:12:48 2017 -0500 Committer: Mike Miller <mmil...@apache.org> Committed: Thu Mar 9 14:38:40 2017 -0500 ---------------------------------------------------------------------- .../java/org/apache/accumulo/shell/Shell.java | 10 ++++--- .../apache/accumulo/shell/ShellConfigTest.java | 25 ++++++----------- shell/src/test/resources/accumulo-site.xml | 28 ++++++++++++++++++++ 3 files changed, 42 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/80373557/shell/src/main/java/org/apache/accumulo/shell/Shell.java ---------------------------------------------------------------------- diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java index 9dc69c5..ccb12a0 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java +++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java @@ -60,6 +60,7 @@ import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; import org.apache.accumulo.core.client.security.tokens.PasswordToken; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.conf.SiteConfiguration; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.data.thrift.TConstraintViolationSummary; @@ -470,7 +471,7 @@ public class Shell extends ShellOptions implements KeywordExecutable { * ClientConfiguration instance * @return The ZooKeepers to connect to */ - static String getZooKeepers(String keepers, ClientConfiguration clientConfig, AccumuloConfiguration conf) { + static String getZooKeepers(String keepers, ClientConfiguration clientConfig) { if (null != keepers) { return keepers; } @@ -479,7 +480,7 @@ public class Shell extends ShellOptions implements KeywordExecutable { return clientConfig.get(ClientProperty.INSTANCE_ZK_HOST); } - return conf.get(Property.INSTANCE_ZK_HOST); + return SiteConfiguration.getInstance(ClientContext.convertClientConfig(clientConfig)).get(Property.INSTANCE_ZK_HOST); } /* @@ -491,9 +492,10 @@ public class Shell extends ShellOptions implements KeywordExecutable { if (instanceName == null) { instanceName = clientConfig.get(ClientProperty.INSTANCE_NAME); } - AccumuloConfiguration conf = ClientContext.convertClientConfig(clientConfig); - String keepers = getZooKeepers(keepersOption, clientConfig, conf); + + String keepers = getZooKeepers(keepersOption, clientConfig); if (instanceName == null) { + AccumuloConfiguration conf = SiteConfiguration.getInstance(ClientContext.convertClientConfig(clientConfig)); Path instanceDir = new Path(VolumeConfiguration.getVolumeUris(conf)[0], "instance_id"); instanceId = UUID.fromString(ZooUtil.getInstanceIDFromHdfs(instanceDir, conf)); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/80373557/shell/src/test/java/org/apache/accumulo/shell/ShellConfigTest.java ---------------------------------------------------------------------- diff --git a/shell/src/test/java/org/apache/accumulo/shell/ShellConfigTest.java b/shell/src/test/java/org/apache/accumulo/shell/ShellConfigTest.java index 49f22a6..7b9f72f 100644 --- a/shell/src/test/java/org/apache/accumulo/shell/ShellConfigTest.java +++ b/shell/src/test/java/org/apache/accumulo/shell/ShellConfigTest.java @@ -27,16 +27,11 @@ import java.io.IOException; import java.io.PrintStream; import java.io.PrintWriter; import java.nio.file.Files; -import java.util.HashMap; -import java.util.Map; import jline.console.ConsoleReader; import org.apache.accumulo.core.client.ClientConfiguration; import org.apache.accumulo.core.client.security.tokens.PasswordToken; -import org.apache.accumulo.core.conf.AccumuloConfiguration; -import org.apache.accumulo.core.conf.ConfigurationCopy; -import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.shell.ShellTest.TestOutputStream; import org.apache.log4j.Level; import org.junit.After; @@ -120,32 +115,28 @@ public class ShellConfigTest { assertTrue(output.get().contains(ParameterException.class.getName())); } + /** + * Tests getting the ZK hosts config value will fail on String parameter, client config and then fall back to Site configuration. SiteConfiguration will get + * the accumulo-site.xml from the classpath in src/test/resources + */ @Test public void testZooKeeperHostFallBackToSite() throws Exception { ClientConfiguration clientConfig = new ClientConfiguration(); - Map<String,String> data = new HashMap<>(); - data.put(Property.INSTANCE_ZK_HOST.getKey(), "site_hostname"); - AccumuloConfiguration conf = new ConfigurationCopy(data); - assertEquals("site_hostname", Shell.getZooKeepers(null, clientConfig, conf)); + assertFalse("Client config contains zk hosts", clientConfig.containsKey(ClientConfiguration.ClientProperty.INSTANCE_ZK_HOST.getKey())); + assertEquals("ShellConfigTestZKHostValue", Shell.getZooKeepers(null, clientConfig)); } @Test public void testZooKeeperHostFromClientConfig() throws Exception { ClientConfiguration clientConfig = new ClientConfiguration(); clientConfig.withZkHosts("cc_hostname"); - Map<String,String> data = new HashMap<>(); - data.put(Property.INSTANCE_ZK_HOST.getKey(), "site_hostname"); - AccumuloConfiguration conf = new ConfigurationCopy(data); - assertEquals("cc_hostname", Shell.getZooKeepers(null, clientConfig, conf)); + assertEquals("cc_hostname", Shell.getZooKeepers(null, clientConfig)); } @Test public void testZooKeeperHostFromOption() throws Exception { ClientConfiguration clientConfig = new ClientConfiguration(); clientConfig.withZkHosts("cc_hostname"); - Map<String,String> data = new HashMap<>(); - data.put(Property.INSTANCE_ZK_HOST.getKey(), "site_hostname"); - AccumuloConfiguration conf = new ConfigurationCopy(data); - assertEquals("opt_hostname", Shell.getZooKeepers("opt_hostname", clientConfig, conf)); + assertEquals("opt_hostname", Shell.getZooKeepers("opt_hostname", clientConfig)); } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/80373557/shell/src/test/resources/accumulo-site.xml ---------------------------------------------------------------------- diff --git a/shell/src/test/resources/accumulo-site.xml b/shell/src/test/resources/accumulo-site.xml new file mode 100644 index 0000000..b468957 --- /dev/null +++ b/shell/src/test/resources/accumulo-site.xml @@ -0,0 +1,28 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<?xml-stylesheet type="text/xsl" href="configuration.xsl"?> + +<!-- Test configuration file used for org.apache.accumulo.shell.ShellConfigTest --> +<configuration> + + <property> + <name>instance.zookeeper.host</name> + <value>ShellConfigTestZKHostValue</value> + </property> + +</configuration>
