----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14972/#review27675 -----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java <https://reviews.apache.org/r/14972/#comment53735> I think I advocated somewhere for removing this method, because its not scaleable for every feature to have a seat at the table in the constructor. The constructor that takes the config object nicely covers this use case. core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java <https://reviews.apache.org/r/14972/#comment53743> Probably do not need to add this method. I think most users connect using the instance name. If they really want to connect using the instance id and pass config, can they use the constructor that only takes config? core/src/main/java/org/apache/accumulo/core/client/impl/ServerClient.java <https://reviews.apache.org/r/14972/#comment53736> Communication failures are the norm, this should probably stay as debug? core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportKey.java <https://reviews.apache.org/r/14972/#comment53737> With authentication via certs isn't this assumption dangerous? core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloOutputFormat.java <https://reviews.apache.org/r/14972/#comment53742> I do not see a code change for OutputConfigurator? I am curious what the behavior is when the instance is also set in client config. core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java <https://reviews.apache.org/r/14972/#comment53738> there is also $ACCUMULO_CONF_DIR (not sure if I have the var name exactly correct) minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java <https://reviews.apache.org/r/14972/#comment53741> add @since tags minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java <https://reviews.apache.org/r/14972/#comment53739> add @since tags minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java <https://reviews.apache.org/r/14972/#comment53746> Mini is intended to be user facing and allow user to test their code against Accumulo. We keep exposing methods in the minicluster public API inorder for Accumulo to test Accumulo. I worry that the more we expose in its API the more we will box ourselves in for future changes to mini (like speeding it up some way). This is a general concern I have, not specific to this change. This change is following the general trend for mini. I have not had time to pursue, but I have wondered if it would be worthwhile to create a mini for internal use and one for external use (with a more minimal API). This may not be worthwhile. minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java <https://reviews.apache.org/r/14972/#comment53740> add @since tags server/src/main/resources/web/flot/jquery.flot.js <https://reviews.apache.org/r/14972/#comment53747> why was this change made? What does it have to do w/ ssl? test/pom.xml <https://reviews.apache.org/r/14972/#comment53749> All export control issues w/ this are resolved? test/src/test/java/org/apache/accumulo/test/ConditionalWriterTest.java <https://reviews.apache.org/r/14972/#comment53750> Whats going on w/ the white space changes in this file? Are you using an incorrect config or was this file checked in w/ incorrect formatting? I suggested some test in this comment https://issues.apache.org/jira/browse/ACCUMULO-1009?focusedCommentId=13771990&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13771990 Do you know if listscans still show the client IP properly with this change? - kturner On Oct. 26, 2013, 2:36 a.m., John Vines wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14972/ > ----------------------------------------------------------- > > (Updated Oct. 26, 2013, 2:36 a.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-1009 > https://issues.apache.org/jira/browse/ACCUMULO-1009 > > > Repository: accumulo > > > Description > ------- > > Michael Berman's October 13 patch for ACCUMULO-1009 > > > Diffs > ----- > > .gitignore 1ffa452 > core/src/main/java/org/apache/accumulo/core/cli/ClientOpts.java 9247d56 > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java > 5b5d041 > > core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java > bb5987d > core/src/main/java/org/apache/accumulo/core/client/impl/MasterClient.java > 32c80f9 > core/src/main/java/org/apache/accumulo/core/client/impl/ServerClient.java > 218bd36 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java > 0376304 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java > 0dd86bf > > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportKey.java > f07139d > > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java > e7dabb5 > > core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloInputFormat.java > bbbd0c3 > > core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloOutputFormat.java > 908b8b3 > > core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloRowInputFormat.java > fe5003b > > core/src/main/java/org/apache/accumulo/core/client/mapred/InputFormatBase.java > c796cd2 > > core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloInputFormat.java > 1cbb606 > > core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java > 727bfec > > core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloRowInputFormat.java > 992990d > > core/src/main/java/org/apache/accumulo/core/client/mapreduce/InputFormatBase.java > 13f9708 > > core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/util/ConfiguratorBase.java > 73405c5 > core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java > 28cb0bd > core/src/main/java/org/apache/accumulo/core/conf/ClientConfiguration.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/conf/Property.java b6fbdd2 > core/src/main/java/org/apache/accumulo/core/security/Credentials.java > 0552e7e > core/src/main/java/org/apache/accumulo/core/security/SecurityUtil.java > 8add1a7 > > core/src/main/java/org/apache/accumulo/core/security/ssl/SslConnectionParams.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java e8dd6a2 > core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java 4140c8c > core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java > cb1f1c8 > > core/src/test/java/org/apache/accumulo/core/conf/ClientConfigurationTest.java > PRE-CREATION > > core/src/test/java/org/apache/accumulo/core/util/shell/ShellSetInstanceTest.java > 23ca13a > > minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java > 77776df > > minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloConfig.java > 0b6c42c > > minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloInstance.java > 540d7ae > > minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterTest.java > 3e749ab > > server/src/main/java/org/apache/accumulo/server/cli/ClientOnDefaultTable.java > 53f5ac2 > > server/src/main/java/org/apache/accumulo/server/cli/ClientOnRequiredTable.java > e9e9bf1 > server/src/main/java/org/apache/accumulo/server/cli/ClientOpts.java 6f3516a > server/src/main/java/org/apache/accumulo/server/client/BulkImporter.java > a04765f > > server/src/main/java/org/apache/accumulo/server/gc/SimpleGarbageCollector.java > 817aa74 > server/src/main/java/org/apache/accumulo/server/util/TServerUtils.java > 1df17fe > server/src/main/resources/web/flot/jquery.flot.js aabc544 > test/pom.xml 8343bb2 > test/src/test/java/org/apache/accumulo/test/ConditionalWriterTest.java > 633ea76 > test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 4fbd293 > test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java > f1a651d > test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 607f2a5 > test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java > c3d3160 > > test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java > c34fff5 > test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java > 69825bc > test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java > 8d58821 > test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java > 4a37d82 > test/src/test/java/org/apache/accumulo/test/functional/SslIT.java > PRE-CREATION > > test/src/test/java/org/apache/accumulo/test/functional/SslWithClientAuthIT.java > PRE-CREATION > test/src/test/java/org/apache/accumulo/test/functional/SslWithJsseIT.java > PRE-CREATION > test/src/test/java/org/apache/accumulo/test/util/CertUtils.java > PRE-CREATION > test/src/test/java/org/apache/accumulo/test/util/CertUtilsTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/14972/diff/ > > > Testing > ------- > > > Thanks, > > John Vines > >
