You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@accumulo.apache.org by Michael Berman <mb...@sqrrl.com> on 2013/11/06 15:57:57 UTC

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

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

(Updated Nov. 6, 2013, 2:57 p.m.)


Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.


Repository: accumulo


Description
-------

(was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)

Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.

Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.


Diffs
-----

  core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
  core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
  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/AbstractInputFormat.java 856936e 
  core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
  core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
  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/mapreduce/AbstractInputFormat.java 626a785 
  core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
  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/lib/util/ConfiguratorBase.java 4f8cdb6 
  core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
  core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
  core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
  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 52e1d04 
  core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
  core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
  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 
  examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
  examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
  minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
  minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
  proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
  server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
  server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
  test/pom.xml 8e4c152 
  test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
  test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
  test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
  test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
  test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
  test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
  test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
  test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
  test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
  test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
  test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
  test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
  test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
  test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
  test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
  test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
  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/util/CertUtils.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/15245/diff/


Testing
-------


Thanks,

Michael Berman

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.
> 
> John Vines wrote:
>     I don't see why that change needs to be a blocker for this patch
> 
> kturner wrote:
>     Its not a blocker.  If this patch goes in I would like to see those methods deprecated.   I can do it if no one else is interested.
> 
> Michael Berman wrote:
>     What's the policy for introducing deprecated references into our own code?  getConfiguration is called in 34 places, from core, gc, master, and server-base.  If we deprecate the method, can we still reference it internally?  Can we just suppress the warnings?
> 
> kturner wrote:
>     We usually remove our usage of deprecated APIs when we deprecate them.  Otherwise it just become technical debt.

Yeah, that's how I think it should work...but in this case it seems like a big change for this late in the process (which is why my reflex is to not even deprecate it for 1.6.0, and attack this issue separately).


- Michael


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by ke...@deenlo.com.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.
> 
> John Vines wrote:
>     I don't see why that change needs to be a blocker for this patch
> 
> kturner wrote:
>     Its not a blocker.  If this patch goes in I would like to see those methods deprecated.   I can do it if no one else is interested.
> 
> Michael Berman wrote:
>     What's the policy for introducing deprecated references into our own code?  getConfiguration is called in 34 places, from core, gc, master, and server-base.  If we deprecate the method, can we still reference it internally?  Can we just suppress the warnings?
> 
> kturner wrote:
>     We usually remove our usage of deprecated APIs when we deprecate them.  Otherwise it just become technical debt.
> 
> Michael Berman wrote:
>     Yeah, that's how I think it should work...but in this case it seems like a big change for this late in the process (which is why my reflex is to not even deprecate it for 1.6.0, and attack this issue separately).

LEt me look into a bit.  I know I want it gone.  I have not looked into what it would take.


- kturner


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.
> 
> John Vines wrote:
>     I don't see why that change needs to be a blocker for this patch
> 
> kturner wrote:
>     Its not a blocker.  If this patch goes in I would like to see those methods deprecated.   I can do it if no one else is interested.
> 
> Michael Berman wrote:
>     What's the policy for introducing deprecated references into our own code?  getConfiguration is called in 34 places, from core, gc, master, and server-base.  If we deprecate the method, can we still reference it internally?  Can we just suppress the warnings?
> 
> kturner wrote:
>     We usually remove our usage of deprecated APIs when we deprecate them.  Otherwise it just become technical debt.
> 
> Michael Berman wrote:
>     Yeah, that's how I think it should work...but in this case it seems like a big change for this late in the process (which is why my reflex is to not even deprecate it for 1.6.0, and attack this issue separately).
> 
> kturner wrote:
>     LEt me look into a bit.  I know I want it gone.  I have not looked into what it would take.

I filed ACCUMULO-1866 to track this issue.  Keith, feel free to assign it to yourself if you end up deciding to do it.  If you don't, I'll take it up later.


- Michael


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


On Nov. 8, 2013, 4:06 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2013, 4:06 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/Constants.java 9db0c40 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/Instance.java 95fc933 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java 6b2a1cf 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java d391464 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java fc45442 
>   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/util/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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java 8f3fa1d 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java 9e6bbe7 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/TestIngest.java 972a20e 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.

I can definitely see an argument for removing
setConfiguration(AccumuloConfiguration) as part of this change, since it's
not clear at all how that interacts (or should interact) with the client
configuration passed into the constructor.  But like John, I also don't see
why changing Instance.getConfiguration() should be tied to this change.  It
existed and has been considered a problem since before this change, and
it's not clear to me that this change makes it any worse.


On Thu, Nov 7, 2013 at 2:08 PM, John Vines <vi...@apache.org> wrote:

>
>
> > On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > >
> core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java,
> line 265
> > > <
> https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> > >
> > >     This should be deprecated since a new client side configuration
> was introduced.  Having the server side configuration here is just
> confusing.   Deprecating it will make it clear that its not intended to be
> used.
> >
> > Michael Berman wrote:
> >     I'm happy to add the @Deprecated annotation, but I firmly believe
> actually removing references to Instance.getConfiguration(), which is
> pervasive in the codebase, should be a tracked separate task.  I'm not sure
> we actually need to go through a deprecation cycle, since it seems like
> there's no value in client code actually using this method...the consensus
> on the old review and conversation in jira and email seems to be that the
> existence of this method on Instance was an error from the beginning and
> should be removed completely.  Is it worth adding the annotation in this
> commit but not doing anything about it (introducing a bunch of deprecation
> warnings elsewhere in the code)?  Or should we just deal with it as part of
> the "remove Instance.getConfiguration" ticket?
> >
> > kturner wrote:
> >     I agree, it should be done as a separate ticket.  I am mostly
> concerned about making a coherent set of API changes for 1.6.   So if its
> done in a separate ticket, I think that ticket should also go in for 1.6 if
> this does.
>
> I don't see why that change needs to be a blocker for this patch
>
>
> - John
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/#review28381
> -----------------------------------------------------------
>
>
> On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/15245/
> > -----------------------------------------------------------
> >
> > (Updated Nov. 6, 2013, 2:57 p.m.)
> >
> >
> > Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines,
> and kturner.
> >
> >
> > Repository: accumulo
> >
> >
> > Description
> > -------
> >
> > (was https://reviews.apache.org/r/14972/ ; created new review created
> so I get responses instead of Vines)
> >
> > Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to
> master), and feedback from the last review integrated.
> >
> > Still need to clean up all the references to newly deprecated ZKInstance
> constructors and Input/OutputFormat config setters, but wanted to get this
> up for feedback sooner.
> >
> >
> > Diffs
> > -----
> >
> >   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56
> >
> core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java
> a85b72e
> >
> 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/AbstractInputFormat.java
> 856936e
> >
> core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java
> cccd7b8
> >
> core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java
> 61838db
> >
> 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/mapreduce/AbstractInputFormat.java
> 626a785
> >
> core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java
> 9ecae53
> >
> 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/lib/util/ConfiguratorBase.java
> 4f8cdb6
> >
> core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
> 0a456ba
> >
> core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java
> PRE-CREATION
> >   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4
> >   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
> 52e1d04
> >
> core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
> cb1f1c8
> >
> core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java
> 62564fa
> >
> 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
> >
> examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java
> ecb42c7
> >
> examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java
> 674d793
> >
> minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java
> 42882bb
> >
> minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java
> bfa7922
> >
> minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java
> 540d7ae
> >
> minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java
> 6470ce1
> >
> minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java
> 26a1546
> >   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b
> >
> server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java
> 53f5ac2
> >
> server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java
> e9e9bf1
> >
> server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java
> 6f3516a
> >
> server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java
> 606941d
> >
> server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java
> 8abd104
> >   test/pom.xml 8e4c152
> >   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741
> >
> test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java
> c6ad74f
> >
> test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java
> 78b2564
> >   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java
> ea6d8ed
> >
> test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java
> e104c99
> >
> test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java
> 6c7cc63
> >   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java
> e6ba77b
> >   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3
> >
> test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java
> d24b85b
> >
> test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java
> d38536a
> >   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java
> 2fb5827
> >
> test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java
> c3d3160
> >
> test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java
> 3f60f1d
> >
> test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java
> 9e42e55
> >   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java
> 7913089
> >   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java
> 8d58821
> >
> test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java
> 9086f13
> >   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/util/CertUtils.java
> PRE-CREATION
> >   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java
> PRE-CREATION
> >
> > Diff: https://reviews.apache.org/r/15245/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Michael Berman
> >
> >
>
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by John Vines <vi...@apache.org>.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.

I don't see why that change needs to be a blocker for this patch


- John


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.
> 
> John Vines wrote:
>     I don't see why that change needs to be a blocker for this patch
> 
> kturner wrote:
>     Its not a blocker.  If this patch goes in I would like to see those methods deprecated.   I can do it if no one else is interested.

What's the policy for introducing deprecated references into our own code?  getConfiguration is called in 34 places, from core, gc, master, and server-base.  If we deprecate the method, can we still reference it internally?  Can we just suppress the warnings?


- Michael


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by ke...@deenlo.com.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?

I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.


- kturner


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by ke...@deenlo.com.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.
> 
> John Vines wrote:
>     I don't see why that change needs to be a blocker for this patch

Its not a blocker.  If this patch goes in I would like to see those methods deprecated.   I can do it if no one else is interested.


- kturner


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by ke...@deenlo.com.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.
> 
> Michael Berman wrote:
>     I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?
> 
> kturner wrote:
>     I agree, it should be done as a separate ticket.  I am mostly concerned about making a coherent set of API changes for 1.6.   So if its done in a separate ticket, I think that ticket should also go in for 1.6 if this does.
> 
> John Vines wrote:
>     I don't see why that change needs to be a blocker for this patch
> 
> kturner wrote:
>     Its not a blocker.  If this patch goes in I would like to see those methods deprecated.   I can do it if no one else is interested.
> 
> Michael Berman wrote:
>     What's the policy for introducing deprecated references into our own code?  getConfiguration is called in 34 places, from core, gc, master, and server-base.  If we deprecate the method, can we still reference it internally?  Can we just suppress the warnings?

We usually remove our usage of deprecated APIs when we deprecate them.  Otherwise it just become technical debt.


- kturner


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line265>
> >
> >     This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.

I'm happy to add the @Deprecated annotation, but I firmly believe actually removing references to Instance.getConfiguration(), which is pervasive in the codebase, should be a tracked separate task.  I'm not sure we actually need to go through a deprecation cycle, since it seems like there's no value in client code actually using this method...the consensus on the old review and conversation in jira and email seems to be that the existence of this method on Instance was an error from the beginning and should be removed completely.  Is it worth adding the annotation in this commit but not doing anything about it (introducing a bunch of deprecation warnings elsewhere in the code)?  Or should we just deal with it as part of the "remove Instance.getConfiguration" ticket?


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 273
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line273>
> >
> >     Also need to deprecate.

This one is only ever called from a single test, I think, so it's easier to deal with than the getter.  I'll try to take care of this one.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/security/ssl/SslConnectionParams.java, line 28
> > <https://reviews.apache.org/r/15245/diff/2/?file=378411#file378411line28>
> >
> >     Is this internal code?  Should it be in the public API?  If not it should go in a different package, like core.impl or something that indicates its for internal use only.  There are already classes in core.security that are part of the API.

Good point.  I'll move it.


> On Nov. 7, 2013, 5:35 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 145
> > <https://reviews.apache.org/r/15245/diff/2/?file=378388#file378388line145>
> >
> >     In the old review there was discussion about passing in a generic commons configuration object and having javadoc point to ClientConfiguration.

Yeah, ok, I think I'm convinced that a sufficiently suggestive javadoc will address my discoverability concern.


- Michael


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by ke...@deenlo.com.

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



core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java
<https://reviews.apache.org/r/15245/#comment55201>

    In the old review there was discussion about passing in a generic commons configuration object and having javadoc point to ClientConfiguration.  



core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java
<https://reviews.apache.org/r/15245/#comment55199>

    This should be deprecated since a new client side configuration was introduced.  Having the server side configuration here is just confusing.   Deprecating it will make it clear that its not intended to be used.



core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java
<https://reviews.apache.org/r/15245/#comment55200>

    Also need to deprecate.



core/src/main/java/org/apache/accumulo/core/security/ssl/SslConnectionParams.java
<https://reviews.apache.org/r/15245/#comment55202>

    Is this internal code?  Should it be in the public API?  If not it should go in a different package, like core.impl or something that indicates its for internal use only.  There are already classes in core.security that are part of the API.


- kturner


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.

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

(Updated Nov. 8, 2013, 4:06 p.m.)


Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.


Changes
-------

Update ClientConfigurationTest per Chris's comment on old review


Repository: accumulo


Description
-------

(was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)

Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.

Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.


Diffs (updated)
-----

  core/src/main/java/org/apache/accumulo/core/Constants.java 9db0c40 
  core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
  core/src/main/java/org/apache/accumulo/core/client/Instance.java 95fc933 
  core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
  core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java 6b2a1cf 
  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/AbstractInputFormat.java 856936e 
  core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
  core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
  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/mapreduce/AbstractInputFormat.java 626a785 
  core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
  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/lib/util/ConfiguratorBase.java 4f8cdb6 
  core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java d391464 
  core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
  core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
  core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
  core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java fc45442 
  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/util/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 52e1d04 
  core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
  core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java 8f3fa1d 
  core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
  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 
  examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
  examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
  minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
  minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
  proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
  server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
  server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java 9e6bbe7 
  server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
  test/pom.xml 8e4c152 
  test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
  test/src/main/java/org/apache/accumulo/test/TestIngest.java 972a20e 
  test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
  test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
  test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
  test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
  test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
  test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
  test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
  test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
  test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
  test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
  test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
  test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
  test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
  test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
  test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
  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/util/CertUtils.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/15245/diff/


Testing
-------


Thanks,

Michael Berman

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.

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

(Updated Nov. 7, 2013, 9:18 p.m.)


Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.


Changes
-------

Updated diff with suggested changes integrated.


Repository: accumulo


Description
-------

(was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)

Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.

Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.


Diffs (updated)
-----

  core/src/main/java/org/apache/accumulo/core/Constants.java 9db0c40 
  core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
  core/src/main/java/org/apache/accumulo/core/client/Instance.java 95fc933 
  core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
  core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java 6b2a1cf 
  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/AbstractInputFormat.java 856936e 
  core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
  core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
  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/mapreduce/AbstractInputFormat.java 626a785 
  core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
  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/lib/util/ConfiguratorBase.java 4f8cdb6 
  core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java d391464 
  core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
  core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
  core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
  core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java fc45442 
  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/util/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 52e1d04 
  core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
  core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java 8f3fa1d 
  core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
  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 
  examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
  examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
  minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
  minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
  minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
  proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
  server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
  server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
  server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java 9e6bbe7 
  server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
  test/pom.xml 8e4c152 
  test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
  test/src/main/java/org/apache/accumulo/test/TestIngest.java 972a20e 
  test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
  test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
  test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
  test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
  test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
  test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
  test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
  test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
  test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
  test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
  test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
  test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
  test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
  test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
  test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
  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/util/CertUtils.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 

Diff: https://reviews.apache.org/r/15245/diff/


Testing
-------


Thanks,

Michael Berman

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Michael Berman <mb...@sqrrl.com>.


> On Nov. 7, 2013, 3:34 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java, line 163
> > <https://reviews.apache.org/r/15245/diff/2/?file=378406#file378406line163>
> >
> >     Might as well lift this array out into a static constant.

That seems like a good idea.  Does it make sense to have a fixed set of environment variables that support substitution in path properties, or should we just support replacing arbitrary $envs?  I'm torn...allowing the environment to affect how paths are resolved in arbitrary ways seems scary and vulnerable, but just having a fixed list seems weirdly restrictive.  Anyone else have an opinion?


- Michael


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


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>

Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/)

Posted by Josh Elser <jo...@gmail.com>.

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



core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java
<https://reviews.apache.org/r/15245/#comment55189>

    Might as well lift this array out into a static constant.


- Josh Elser


On Nov. 6, 2013, 2:57 p.m., Michael Berman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15245/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2013, 2:57 p.m.)
> 
> 
> Review request for accumulo, Sean Busbey, Christopher Tubbs, John Vines, and kturner.
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> (was https://reviews.apache.org/r/14972/ ; created new review created so I get responses instead of Vines)
> 
> Rebased on 1.6.0-SNAPSHOT's current HEAD (which is pretty close to master), and feedback from the last review integrated.
> 
> Still need to clean up all the references to newly deprecated ZKInstance constructors and Input/OutputFormat config setters, but wanted to get this up for feedback sooner.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 
>   core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java a85b72e 
>   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/AbstractInputFormat.java 856936e 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java cccd7b8 
>   core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloMultiTableInputFormat.java 61838db 
>   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/mapreduce/AbstractInputFormat.java 626a785 
>   core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java 9ecae53 
>   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/lib/util/ConfiguratorBase.java 4f8cdb6 
>   core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java 0a456ba 
>   core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java b915ee4 
>   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 52e1d04 
>   core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java cb1f1c8 
>   core/src/test/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBaseTest.java 62564fa 
>   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 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/FileDataQuery.java ecb42c7 
>   examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TokenFileWordCount.java 674d793 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java 42882bb 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java bfa7922 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java 540d7ae 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java 6470ce1 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java 26a1546 
>   proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java c92f73b 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java 53f5ac2 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java e9e9bf1 
>   server/base/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a 
>   server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java 606941d 
>   server/base/src/main/java/org/apache/accumulo/server/util/TServerUtils.java 8abd104 
>   test/pom.xml 8e4c152 
>   test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java 948a741 
>   test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java c6ad74f 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java 78b2564 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java ea6d8ed 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTool.java e104c99 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerifyTool.java 6c7cc63 
>   test/src/main/java/org/apache/accumulo/test/scalability/ScaleTest.java e6ba77b 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 31867f3 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java d24b85b 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java d38536a 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 2fb5827 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c3d3160 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 3f60f1d 
>   test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java 9e42e55 
>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java 7913089 
>   test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java 8d58821 
>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java 9086f13 
>   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/util/CertUtils.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15245/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Berman
> 
>