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 Mon, 06 Oct 2014 18:11:56 GMT


> On Oct. 3, 2014, 2:38 p.m., Christopher Tubbs wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/NewTableConfiguration.java,
lines 46-53
> > <https://reviews.apache.org/r/26188/diff/1/?file=713518#file713518line46>
> >
> >     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();

After looking at the current table creation options, it seems that from the above, the only
ones needed to carry over from the existing features, are the methods to set the time type
and choose to omit the default iterators (I'm in favor of referring to the default iterators
generically, rather than have specific methods for the versioning iterator, to prevent the
API from getting bloated if we decide to add more default iterators that make sense.)

Everything else I listed here can be added later, as follow-on issues to improve the NewTableConfiguration.

I'm not too stuck on the builder-pattern naming. Standard setters/getters are probably the
simplest to deal with for now. We can defer the builder pattern to ACCUMULO-1904.


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26188/#review55372
-----------------------------------------------------------


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