accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 14972: ACCUMULO-1009 - add use of SSL for thrift comms
Date Tue, 05 Nov 2013 22:22:39 GMT


> On Nov. 1, 2013, 9:37 p.m., Christopher Tubbs wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line
194
> > <https://reviews.apache.org/r/14972/diff/1/?file=371853#file371853line194>
> >
> >     Can we make this method signature, and others like it, take just a Configuration
object (commons Configuration) rather than an Accumulo-specific one? I'd rather not be tied
to our interface when we could be more general and allow users to construct configuration
by all the options available to them using that standard library.
> 
> John Vines wrote:
>     That sounds fairly utilitarian, but having the dedicated builder methods we get from
having a dedicated Configuration class will make it substantially easier for users to use
the configuration and explore the options we provide to them. They could theoretically use
the ClientConfiguration with a Configuration API, but then it makes it harder for the user
to find it. It may be a little bit redundant, but I think having 2 signatures, one for Configuration
and one for ClientConfiguration, to make it easier for the user to find things.

I think we can have a constructor that take a a generic config object and still make it easy
for users to find something w/ helper methods via java doc.  Something like the following.

class ClientConfiguration extends Configuration {
  //lots of convient methods specific to accumulo config
}

class ZookeeperInstance {
/**
 * @param config see {@link ClientConfiguration} it extends Configuration and offer convenience
methods specific to Accumulo
 */
 ZookeeperInstance(Configuration config)
}


> On Nov. 1, 2013, 9:37 p.m., Christopher Tubbs wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, lines
313-319
> > <https://reviews.apache.org/r/14972/diff/1/?file=371853#file371853line313>
> >
> >     This method really needs to be deprecated. It never made sense to return an
AccumuloConfiguration here, because AccumuloConfiguration isn't for clients.

Agreed, and thats probably a separate ticket?


- kturner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14972/#review28034
-----------------------------------------------------------


On Oct. 31, 2013, 2:35 p.m., John Vines wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14972/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 2:35 p.m.)
> 
> 
> Review request for accumulo and Michael Berman.
> 
> 
> Bugs: ACCUMULO-1009
>     https://issues.apache.org/jira/browse/ACCUMULO-1009
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Michael Berman's October 13 patch for ACCUMULO-1009
> 
> 
> Diffs
> -----
> 
>   .gitignore 1ffa452 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java 5b5d041 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java
bb5987d 
>   core/src/main/java/org/apache/accumulo/core/client/impl/MasterClient.java 32c80f9 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ServerClient.java 218bd36 
>   core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java
0376304 
>   core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java
0dd86bf 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportKey.java f07139d

>   core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java e7dabb5

>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java
bbbd0c3 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloOutputFormat.java
908b8b3 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloRowInputFormat.java
fe5003b 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/InputFormatBase.java c796cd2

>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java
1cbb606 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java
727bfec 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloRowInputFormat.java
992990d 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/InputFormatBase.java 13f9708

>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBase.java
73405c5 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 28cb0bd

>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b6fbdd2 
>   core/src/main/java/org/apache/accumulo/core/security/Credentials.java 0552e7e 
>   core/src/main/java/org/apache/accumulo/core/security/SecurityUtil.java 8add1a7 
>   core/src/main/java/org/apache/accumulo/core/security/ssl/SslConnectionParams.java PRE-CREATION

>   core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java e8dd6a2 
>   core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java 4140c8c 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8

>   core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java PRE-CREATION

>   core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java 23ca13a

>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java
77776df 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java 0b6c42c

>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java
3e749ab 
>   server/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2

>   server/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1

>   server/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java a04765f 
>   server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java 817aa74

>   server/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 1df17fe 
>   server/src/main/resources/web/flot/jquery.flot.js aabc544 
>   test/pom.xml 8343bb2 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterTest.java 633ea76 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 4fbd293 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java f1a651d 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 607f2a5 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java c34fff5

>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 69825bc 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 4a37d82 
>   test/src/test/java/org/apache/accumulo/test/functional/SslIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/SslWithClientAuthIT.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/test/functional/SslWithJsseIT.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/test/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14972/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> John Vines
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message