accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Christopher Tubbs" <ctubbsii...@apache.org>
Subject Re: Review Request 26188: ACCUMULO-3176 Add ability to create a table with user specified initial properties
Date Fri, 03 Oct 2014 18:38:29 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message