----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26188/#review55372 -----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/client/impl/NewTableConfiguration.java <https://reviews.apache.org/r/26188/#comment95732> This shouldn't be in the impl package if it's intended to be part of the public API. core/src/main/java/org/apache/accumulo/core/client/impl/NewTableConfiguration.java <https://reviews.apache.org/r/26188/#comment95736> Needs javadoc core/src/main/java/org/apache/accumulo/core/client/impl/NewTableConfiguration.java <https://reviews.apache.org/r/26188/#comment95739> This configuration class needs additional options, and might benefit (in terms of readability, and fluent usage) from builder-pattern naming conventions instead of setter/getter naming conventions. Suggestions below, for discussion: withoutDefaultConstraints(); withConstraint(TableConstraint constraint); withIterator(IteratorSetting iterator); withoutDefaultIterators(); // replaces the limit version methods withTimeType(TimeType tt); withAdditionalProperties(Map<String, String> props); // asOffline(); // for future, see ACCUMULO-1904 Alternatively, the setter/getter terminology could stay, but with options to remove defaults: removeDefaultConstraints(); removeDefaultIterators(); core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java <https://reviews.apache.org/r/26188/#comment95733> Look at the Guava Preconditions class. It has utilities to do this check and throw an IllegalArgumentException with a provided message in a one-liner. core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java <https://reviews.apache.org/r/26188/#comment95734> Another opportunity for Preconditions. - Christopher Tubbs On Oct. 3, 2014, 1:30 p.m., Jenna Huston wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26188/ > ----------------------------------------------------------- > > (Updated Oct. 3, 2014, 1:30 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-3176 > https://issues.apache.org/jira/browse/ACCUMULO-3176 > > > Repository: accumulo > > > Description > ------- > > Gives the ability to add properties to tables before they are initialized. > Therefore these properties will take effect before the default tablet is > created. We create a NewTableConfiguration class and send that in the create > method as opposed to adding another method. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java > 97f538d > > core/src/main/java/org/apache/accumulo/core/client/impl/NewTableConfiguration.java > PRE-CREATION > > core/src/main/java/org/apache/accumulo/core/client/impl/TableOperationsImpl.java > e46b9c9 > core/src/main/java/org/apache/accumulo/core/client/mock/MockAccumulo.java > 32dbb28 > core/src/main/java/org/apache/accumulo/core/client/mock/MockTable.java > 35cbdd2 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockTableOperations.java > 08750fe > > core/src/test/java/org/apache/accumulo/core/client/impl/TableOperationsHelperTest.java > 02838ed > proxy/src/main/java/org/apache/accumulo/proxy/ProxyServer.java a778add > > shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java > 81b39d2 > > test/src/test/java/org/apache/accumulo/test/CreateTableWithNewTableConfigIT.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/26188/diff/ > > > Testing > ------- > > New IT, ran unit test and integration tests > > > Thanks, > > Jenna Huston > >
