You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2018/05/25 20:57:32 UTC

[GitHub] mikewalch closed pull request #500: #408 - Removed ClientConfiguration

mikewalch closed pull request #500:  #408 - Removed ClientConfiguration
URL: https://github.com/apache/accumulo/pull/500
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientConfConverter.java b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientConfConverter.java
index 7bdddf0d8a..4d1306bd0f 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientConfConverter.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientConfConverter.java
@@ -302,5 +302,4 @@ public void getProperties(Map<String,String> props, Predicate<String> filter) {
     }
     return clientConf;
   }
-
 }
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
index 028480a8cf..2d19bf59e4 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ClientContext.java
@@ -26,12 +26,12 @@
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
 import org.apache.accumulo.core.client.BatchWriterConfig;
-import org.apache.accumulo.core.client.ClientConfiguration;
 import org.apache.accumulo.core.client.ConnectionInfo;
 import org.apache.accumulo.core.client.Connector;
 import org.apache.accumulo.core.client.Instance;
 import org.apache.accumulo.core.client.ZooKeeperInstance;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
+import org.apache.accumulo.core.conf.ClientProperty;
 import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.core.rpc.SaslConnectionParams;
 import org.apache.accumulo.core.rpc.SslConnectionParams;
@@ -52,7 +52,7 @@
 
   protected final Instance inst;
   private Credentials creds;
-  private ClientConfiguration clientConf;
+  private Properties clientProps;
   private BatchWriterConfig batchWriterConfig = new BatchWriterConfig();
   private final AccumuloConfiguration rpcConf;
   protected Connector conn;
@@ -71,24 +71,19 @@
 
   public ClientContext(ConnectionInfo connectionInfo) {
     this(ConnectionInfoFactory.getInstance(connectionInfo),
-        ConnectionInfoFactory.getCredentials(connectionInfo),
-        ConnectionInfoFactory.getClientConfiguration(connectionInfo),
+        ConnectionInfoFactory.getCredentials(connectionInfo), connectionInfo.getProperties(),
         ConnectionInfoFactory.getBatchWriterConfig(connectionInfo));
   }
 
   public ClientContext(Instance instance, Credentials credentials, Properties clientProps) {
-    this(instance, credentials, ClientConfConverter.toClientConf(clientProps));
+    this(instance, credentials, clientProps, new BatchWriterConfig());
   }
 
-  public ClientContext(Instance instance, Credentials credentials, ClientConfiguration clientConf) {
-    this(instance, credentials, clientConf, new BatchWriterConfig());
-  }
-
-  public ClientContext(Instance instance, Credentials credentials, ClientConfiguration clientConf,
+  public ClientContext(Instance instance, Credentials credentials, Properties clientProps,
       BatchWriterConfig batchWriterConfig) {
     this(instance, credentials,
-        ClientConfConverter.toAccumuloConf(requireNonNull(clientConf, "clientConf is null")));
-    this.clientConf = clientConf;
+        ClientConfConverter.toAccumuloConf(requireNonNull(clientProps, "clientProps is null")));
+    this.clientProps = clientProps;
     this.batchWriterConfig = batchWriterConfig;
   }
 
@@ -101,16 +96,15 @@ public ClientContext(Instance instance, Credentials credentials,
     inst = requireNonNull(instance, "instance is null");
     creds = requireNonNull(credentials, "credentials is null");
     rpcConf = requireNonNull(serverConf, "serverConf is null");
-    clientConf = null;
+    clientProps = null;
 
     saslSupplier = () -> {
-      // Use the clientConf if we have it
-      if (null != clientConf) {
-        if (!clientConf.hasSasl()) {
+      // Use the clientProps if we have it
+      if (null != clientProps) {
+        if (!ClientProperty.SASL_ENABLED.getBoolean(clientProps)) {
           return null;
         }
-        return new SaslConnectionParams(ClientConfConverter.toProperties(clientConf),
-            getCredentials().getToken());
+        return new SaslConnectionParams(clientProps, getCredentials().getToken());
       }
       AccumuloConfiguration conf = getConfiguration();
       if (!conf.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)) {
@@ -123,7 +117,6 @@ public ClientContext(Instance instance, Credentials credentials,
         () -> getConfiguration().getTimeInMillis(Property.GENERAL_RPC_TIMEOUT));
     sslSupplier = memoizeWithExpiration(() -> SslConnectionParams.forClient(getConfiguration()));
     saslSupplier = memoizeWithExpiration(saslSupplier);
-
   }
 
   /**
@@ -134,7 +127,7 @@ public Instance getInstance() {
   }
 
   public ConnectionInfo getConnectionInfo() {
-    return new ConnectionInfoImpl(ClientConfConverter.toProperties(clientConf), creds.getToken());
+    return new ConnectionInfoImpl(clientProps, creds.getToken());
   }
 
   /**
@@ -217,5 +210,4 @@ public synchronized TCredentials rpcCreds() {
 
     return rpcCreds;
   }
-
 }
diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ConnectionInfoFactory.java b/core/src/main/java/org/apache/accumulo/core/client/impl/ConnectionInfoFactory.java
index b7ec1372f1..0bbde61796 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/impl/ConnectionInfoFactory.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ConnectionInfoFactory.java
@@ -43,15 +43,11 @@ public static Long getLong(ConnectionInfo info, ClientProperty property) {
 
   public static Connector getConnector(ConnectionInfo info)
       throws AccumuloSecurityException, AccumuloException {
-    return new ConnectorImpl(getClientContext(info));
-  }
-
-  public static ClientContext getClientContext(ConnectionInfo info) {
-    return new ClientContext(info);
+    return new ConnectorImpl(new ClientContext(info));
   }
 
   public static Instance getInstance(ConnectionInfo info) {
-    return new ZooKeeperInstance(getClientConfiguration(info));
+    return new ZooKeeperInstance(ClientConfConverter.toClientConf(info.getProperties()));
   }
 
   public static Credentials getCredentials(ConnectionInfo info) {
@@ -78,10 +74,4 @@ public static BatchWriterConfig getBatchWriterConfig(ConnectionInfo info) {
     }
     return batchWriterConfig;
   }
-
-  @SuppressWarnings("deprecation")
-  public static org.apache.accumulo.core.client.ClientConfiguration getClientConfiguration(
-      ConnectionInfo info) {
-    return ClientConfConverter.toClientConf(info.getProperties());
-  }
 }
diff --git a/core/src/test/java/org/apache/accumulo/core/client/ZooKeeperInstanceTest.java b/core/src/test/java/org/apache/accumulo/core/client/ZooKeeperInstanceTest.java
index 467a4a9f97..877ddb9475 100644
--- a/core/src/test/java/org/apache/accumulo/core/client/ZooKeeperInstanceTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/client/ZooKeeperInstanceTest.java
@@ -26,7 +26,6 @@
 import java.util.UUID;
 
 import org.apache.accumulo.core.Constants;
-import org.apache.accumulo.core.client.ClientConfiguration.ClientProperty;
 import org.apache.accumulo.fate.zookeeper.ZooCache;
 import org.apache.accumulo.fate.zookeeper.ZooCacheFactory;
 import org.easymock.EasyMock;
@@ -40,18 +39,29 @@
   private ZooCache zc;
   private ZooKeeperInstance zki;
 
-  private void mockIdConstruction(@SuppressWarnings("deprecation") ClientConfiguration config) {
-    expect(config.get(ClientProperty.INSTANCE_ID)).andReturn(IID_STRING);
-    expect(config.get(ClientProperty.INSTANCE_NAME)).andReturn(null);
-    expect(config.get(ClientProperty.INSTANCE_ZK_HOST)).andReturn("zk1");
-    expect(config.get(ClientProperty.INSTANCE_ZK_TIMEOUT)).andReturn("30");
+  @SuppressWarnings("deprecation")
+  private static org.apache.accumulo.core.client.ClientConfiguration.ClientProperty INSTANCE_ID = org.apache.accumulo.core.client.ClientConfiguration.ClientProperty.INSTANCE_ID;
+  @SuppressWarnings("deprecation")
+  private static org.apache.accumulo.core.client.ClientConfiguration.ClientProperty INSTANCE_NAME = org.apache.accumulo.core.client.ClientConfiguration.ClientProperty.INSTANCE_NAME;
+  @SuppressWarnings("deprecation")
+  private static org.apache.accumulo.core.client.ClientConfiguration.ClientProperty INSTANCE_ZK_HOST = org.apache.accumulo.core.client.ClientConfiguration.ClientProperty.INSTANCE_ZK_HOST;
+  @SuppressWarnings("deprecation")
+  private static org.apache.accumulo.core.client.ClientConfiguration.ClientProperty INSTANCE_ZK_TIMEOUT = org.apache.accumulo.core.client.ClientConfiguration.ClientProperty.INSTANCE_ZK_TIMEOUT;
+
+  @SuppressWarnings("deprecation")
+  private void mockIdConstruction(ClientConfiguration config) {
+    expect(config.get(INSTANCE_ID)).andReturn(IID_STRING);
+    expect(config.get(INSTANCE_NAME)).andReturn(null);
+    expect(config.get(INSTANCE_ZK_HOST)).andReturn("zk1");
+    expect(config.get(INSTANCE_ZK_TIMEOUT)).andReturn("30");
   }
 
-  private void mockNameConstruction(@SuppressWarnings("deprecation") ClientConfiguration config) {
-    expect(config.get(ClientProperty.INSTANCE_ID)).andReturn(null);
-    expect(config.get(ClientProperty.INSTANCE_NAME)).andReturn("instance");
-    expect(config.get(ClientProperty.INSTANCE_ZK_HOST)).andReturn("zk1");
-    expect(config.get(ClientProperty.INSTANCE_ZK_TIMEOUT)).andReturn("30");
+  @SuppressWarnings("deprecation")
+  private void mockNameConstruction(ClientConfiguration config) {
+    expect(config.get(INSTANCE_ID)).andReturn(null);
+    expect(config.get(INSTANCE_NAME)).andReturn("instance");
+    expect(config.get(INSTANCE_ZK_HOST)).andReturn("zk1");
+    expect(config.get(INSTANCE_ZK_TIMEOUT)).andReturn("30");
   }
 
   @Before
@@ -72,23 +82,23 @@ public void setUp() {
   }
 
   @Test(expected = IllegalArgumentException.class)
+  @SuppressWarnings("deprecation")
   public void testInvalidConstruction() {
-    @SuppressWarnings("deprecation")
     ClientConfiguration config = createMock(ClientConfiguration.class);
-    expect(config.get(ClientProperty.INSTANCE_ID)).andReturn(IID_STRING);
+    expect(config.get(INSTANCE_ID)).andReturn(IID_STRING);
     mockNameConstruction(config);
     replay(config);
     new ZooKeeperInstance(config);
   }
 
   @Test(expected = IllegalArgumentException.class)
+  @SuppressWarnings("deprecation")
   public void testInvalidConstruction2() {
-    @SuppressWarnings("deprecation")
     ClientConfiguration config = createMock(ClientConfiguration.class);
-    expect(config.get(ClientProperty.INSTANCE_ID)).andReturn(null);
-    expect(config.get(ClientProperty.INSTANCE_NAME)).andReturn(null);
-    expect(config.get(ClientProperty.INSTANCE_ZK_HOST)).andReturn("zk1");
-    expect(config.get(ClientProperty.INSTANCE_ZK_TIMEOUT)).andReturn("30");
+    expect(config.get(INSTANCE_ID)).andReturn(null);
+    expect(config.get(INSTANCE_NAME)).andReturn(null);
+    expect(config.get(INSTANCE_ZK_HOST)).andReturn("zk1");
+    expect(config.get(INSTANCE_ZK_TIMEOUT)).andReturn("30");
     replay(config);
     new ZooKeeperInstance(config);
   }
diff --git a/core/src/test/java/org/apache/accumulo/core/client/impl/ClientContextTest.java b/core/src/test/java/org/apache/accumulo/core/client/impl/ClientContextTest.java
index 648bfb23de..4244ed68c7 100644
--- a/core/src/test/java/org/apache/accumulo/core/client/impl/ClientContextTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/client/impl/ClientContextTest.java
@@ -20,8 +20,8 @@
 import java.net.URL;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Properties;
 
-import org.apache.accumulo.core.client.ClientConfiguration;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
 import org.apache.accumulo.core.conf.CredentialProviderFactoryShim;
 import org.apache.accumulo.core.conf.Property;
@@ -67,10 +67,9 @@ public void loadSensitivePropertyFromCredentialProvider() {
     }
 
     String absPath = getKeyStoreUrl(keystore);
-    ClientConfiguration clientConf = ClientConfiguration.create()
-        .with(Property.GENERAL_SECURITY_CREDENTIAL_PROVIDER_PATHS.getKey(), absPath);
-
-    AccumuloConfiguration accClientConf = ClientConfConverter.toAccumuloConf(clientConf);
+    Properties props = new Properties();
+    props.setProperty(Property.GENERAL_SECURITY_CREDENTIAL_PROVIDER_PATHS.getKey(), absPath);
+    AccumuloConfiguration accClientConf = ClientConfConverter.toAccumuloConf(props);
     Assert.assertEquals("mysecret", accClientConf.get(Property.INSTANCE_SECRET));
   }
 
@@ -79,10 +78,8 @@ public void defaultValueForSensitiveProperty() {
     if (!isCredentialProviderAvailable) {
       return;
     }
-
-    ClientConfiguration clientConf = ClientConfiguration.create();
-
-    AccumuloConfiguration accClientConf = ClientConfConverter.toAccumuloConf(clientConf);
+    Properties props = new Properties();
+    AccumuloConfiguration accClientConf = ClientConfConverter.toAccumuloConf(props);
     Assert.assertEquals(Property.INSTANCE_SECRET.getDefaultValue(),
         accClientConf.get(Property.INSTANCE_SECRET));
   }
@@ -94,10 +91,10 @@ public void sensitivePropertiesIncludedInProperties() {
     }
 
     String absPath = getKeyStoreUrl(keystore);
-    ClientConfiguration clientConf = ClientConfiguration.create()
-        .with(Property.GENERAL_SECURITY_CREDENTIAL_PROVIDER_PATHS.getKey(), absPath);
+    Properties clientProps = new Properties();
+    clientProps.setProperty(Property.GENERAL_SECURITY_CREDENTIAL_PROVIDER_PATHS.getKey(), absPath);
 
-    AccumuloConfiguration accClientConf = ClientConfConverter.toAccumuloConf(clientConf);
+    AccumuloConfiguration accClientConf = ClientConfConverter.toAccumuloConf(clientProps);
     Map<String,String> props = new HashMap<>();
     accClientConf.getProperties(props, x -> true);
 
diff --git a/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloFileOutputFormatIT.java b/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloFileOutputFormatIT.java
index 83555ca7ea..2db6fc3e8f 100644
--- a/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloFileOutputFormatIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloFileOutputFormatIT.java
@@ -150,8 +150,7 @@ public int run(String[] args) throws Exception {
 
       job.setInputFormat(AccumuloInputFormat.class);
 
-      AccumuloInputFormat.setZooKeeperInstance(job, getCluster().getClientConfig());
-      AccumuloInputFormat.setConnectorInfo(job, getAdminPrincipal(), getAdminToken());
+      AccumuloInputFormat.setConnectionInfo(job, getConnectionInfo());
       AccumuloInputFormat.setInputTableName(job, table);
       AccumuloFileOutputFormat.setOutputPath(job, new Path(args[1]));
       AccumuloFileOutputFormat.setSampler(job, SAMPLER_CONFIG);
diff --git a/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloMultiTableInputFormatIT.java b/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloMultiTableInputFormatIT.java
index 7beb021e8b..be5ad80429 100644
--- a/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloMultiTableInputFormatIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloMultiTableInputFormatIT.java
@@ -31,7 +31,6 @@
 import org.apache.accumulo.core.client.mapred.AccumuloMultiTableInputFormat;
 import org.apache.accumulo.core.client.mapred.RangeInputSplit;
 import org.apache.accumulo.core.client.mapreduce.InputTableConfig;
-import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Mutation;
 import org.apache.accumulo.core.data.Value;
@@ -97,8 +96,6 @@ public int run(String[] args) throws Exception {
             "Usage : " + MRTester.class.getName() + " <table1> <table2>");
       }
 
-      String user = getAdminPrincipal();
-      AuthenticationToken pass = getAdminToken();
       String table1 = args[0];
       String table2 = args[1];
 
@@ -107,8 +104,7 @@ public int run(String[] args) throws Exception {
 
       job.setInputFormat(AccumuloInputFormat.class);
 
-      AccumuloMultiTableInputFormat.setConnectorInfo(job, user, pass);
-      AccumuloMultiTableInputFormat.setZooKeeperInstance(job, getCluster().getClientConfig());
+      AccumuloMultiTableInputFormat.setConnectionInfo(job, getConnectionInfo());
 
       InputTableConfig tableConfig1 = new InputTableConfig();
       InputTableConfig tableConfig2 = new InputTableConfig();
diff --git a/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloRowInputFormatIT.java b/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloRowInputFormatIT.java
index a66cd18a94..4cd772632c 100644
--- a/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloRowInputFormatIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/mapred/AccumuloRowInputFormatIT.java
@@ -31,9 +31,7 @@
 import org.apache.accumulo.core.client.BatchWriterConfig;
 import org.apache.accumulo.core.client.Connector;
 import org.apache.accumulo.core.client.MutationsRejectedException;
-import org.apache.accumulo.core.client.mapred.AccumuloInputFormat;
 import org.apache.accumulo.core.client.mapred.AccumuloRowInputFormat;
-import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.KeyValue;
 import org.apache.accumulo.core.data.Mutation;
@@ -154,8 +152,6 @@ public int run(String[] args) throws Exception {
         throw new IllegalArgumentException("Usage : " + MRTester.class.getName() + " <table>");
       }
 
-      String user = getAdminPrincipal();
-      AuthenticationToken pass = getAdminToken();
       String table = args[0];
 
       JobConf job = new JobConf(getConf());
@@ -163,9 +159,8 @@ public int run(String[] args) throws Exception {
 
       job.setInputFormat(AccumuloRowInputFormat.class);
 
-      AccumuloInputFormat.setConnectorInfo(job, user, pass);
-      AccumuloInputFormat.setInputTableName(job, table);
-      AccumuloRowInputFormat.setZooKeeperInstance(job, getCluster().getClientConfig());
+      AccumuloRowInputFormat.setConnectionInfo(job, getConnectionInfo());
+      AccumuloRowInputFormat.setInputTableName(job, table);
 
       job.setMapperClass(TestMapper.class);
       job.setMapOutputKeyClass(Key.class);
diff --git a/test/src/main/java/org/apache/accumulo/test/mapred/TokenFileIT.java b/test/src/main/java/org/apache/accumulo/test/mapred/TokenFileIT.java
index 4f569b7c28..dbe3f1fc3a 100644
--- a/test/src/main/java/org/apache/accumulo/test/mapred/TokenFileIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/mapred/TokenFileIT.java
@@ -91,6 +91,7 @@ public void close() throws IOException {
 
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public int run(String[] args) throws Exception {
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services