> 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 > >
