Return-Path: X-Original-To: apmail-accumulo-dev-archive@www.apache.org Delivered-To: apmail-accumulo-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 5951F10258 for ; Thu, 7 Nov 2013 19:08:58 +0000 (UTC) Received: (qmail 37117 invoked by uid 500); 7 Nov 2013 19:08:58 -0000 Delivered-To: apmail-accumulo-dev-archive@accumulo.apache.org Received: (qmail 37076 invoked by uid 500); 7 Nov 2013 19:08:58 -0000 Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list dev@accumulo.apache.org Received: (qmail 37066 invoked by uid 500); 7 Nov 2013 19:08:58 -0000 Delivered-To: apmail-incubator-accumulo-dev@incubator.apache.org Received: (qmail 37054 invoked by uid 99); 7 Nov 2013 19:08:58 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 07 Nov 2013 19:08:58 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 242811D2F25; Thu, 7 Nov 2013 19:08:55 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6913150941347948706==" MIME-Version: 1.0 Subject: Re: Review Request 15245: ACCUMULO-1009 - add use of SSL for thrift comms (was https://reviews.apache.org/r/14972/) From: "John Vines" To: "Christopher Tubbs" , "John Vines" , "Sean Busbey" , keith@deenlo.com Cc: "Michael Berman" , "accumulo" Date: Thu, 07 Nov 2013 19:08:55 -0000 Message-ID: <20131107190855.30022.70120@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "John Vines" X-ReviewGroup: accumulo X-ReviewRequest-URL: https://reviews.apache.org/r/15245/ X-Sender: "John Vines" References: <20131107173512.30021.36184@reviews.apache.org> In-Reply-To: <20131107173512.30021.36184@reviews.apache.org> Reply-To: "John Vines" --===============6913150941347948706== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Nov. 7, 2013, 5:35 p.m., kturner wrote: > > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java, line 265 > > > > > > 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 > > --===============6913150941347948706==--