[GitHub] keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public API

2017-10-18 Thread GitBox 
keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public 
API
URL: https://github.com/apache/accumulo-testing/pull/7#discussion_r145549177
 
 

 ##
 File path: 
core/src/main/java/org/apache/accumulo/testing/core/randomwalk/shard/BulkInsert.java
 ##
 @@ -172,7 +171,7 @@ private void sort(State state, RandWalkEnv env, FileSystem 
fs, String tableName,
 
 Collection splits = conn.tableOperations().listSplits(tableName, 
maxSplits);
 for (Text split : splits)
-  
out.println(Base64.getEncoder().encodeToString(TextUtil.getBytes(split)));
+  out.println(Base64.getEncoder().encodeToString(split.getBytes()));
 
 Review comment:
   Text has a copyBytes() method.  Would probably be best to use that here.  I 
think TextUtil method only copies if needed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public API

2017-10-18 Thread GitBox 
keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public 
API
URL: https://github.com/apache/accumulo-testing/pull/7#discussion_r145509955
 
 

 ##
 File path: 
core/src/main/java/org/apache/accumulo/testing/core/randomwalk/shard/BulkInsert.java
 ##
 @@ -172,7 +171,7 @@ private void sort(State state, RandWalkEnv env, FileSystem 
fs, String tableName,
 
 Collection splits = conn.tableOperations().listSplits(tableName, 
maxSplits);
 for (Text split : splits)
-  
out.println(Base64.getEncoder().encodeToString(TextUtil.getBytes(split)));
+  out.println(Base64.getEncoder().encodeToString(split.getBytes()));
 
 Review comment:
   Be careful here.  Text has length.  The getBytes() method may return the 
internal byte array, which may be longer than the length.  I think this is the 
reason that TextUtil.getBytes() exists.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public API

2017-10-18 Thread GitBox 
keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public 
API
URL: https://github.com/apache/accumulo-testing/pull/7#discussion_r145510831
 
 

 ##
 File path: 
core/src/main/java/org/apache/accumulo/testing/core/randomwalk/concurrent/Config.java
 ##
 @@ -190,10 +186,8 @@ private void changeTableSetting(RandomDataGenerator 
random, State state, RandWal
 try {
   env.getAccumuloConnector().tableOperations().setProperty(table, 
setting.property.getKey(), "" + newValue);
 } catch (AccumuloException ex) {
-  if (ex.getCause() instanceof ThriftTableOperationException) {
 
 Review comment:
   How did you find what exception to replace the cause with?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public API

2017-10-18 Thread GitBox 
keith-turner commented on a change in pull request #7: ACCUMULO-4717 Use public 
API
URL: https://github.com/apache/accumulo-testing/pull/7#discussion_r145510442
 
 

 ##
 File path: 
core/src/main/java/org/apache/accumulo/testing/core/randomwalk/image/ImageFixture.java
 ##
 @@ -64,15 +63,15 @@ public void setUp(State state, RandWalkEnv env) throws 
Exception {
 try {
   conn.tableOperations().create(imageTableName);
   conn.tableOperations().addSplits(imageTableName, splits);
-  log.debug("Created table " + imageTableName + " (id:" + 
Tables.getNameToIdMap(instance).get(imageTableName) + ")");
 
 Review comment:
   I think there are public apis to get the table id


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services