Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo >> mail # dev >> Review Request 16224: ACCUMULO-1986 - data integrity checks for Key and Mutation Thrift constructors


Copy link to this message
-
Re: Review Request 16224: ACCUMULO-1986 - data integrity checks for Key and Mutation Thrift constructors


> On Dec. 12, 2013, 9:40 p.m., Eric Newton wrote:
> > src/core/src/main/java/org/apache/accumulo/core/data/Key.java, line 295
> > <https://reviews.apache.org/r/16224/diff/1/?file=397000#file397000line295>
> >
> >     Check out o.a.a.c.u.ArgumentChecker for null value checks.
>
> Bill Havanki wrote:
>     Ugh, the checks are nice but their exception messages aren't customizable, e.g., "argument 'row' was null". I could enhance ArgumentChecker, if you think it's important enough to do that in this ticket.
>
> Josh Elser wrote:
>     Agreed, Bill. ArgumentChecker is nicer than nothing, but Guava's Preconditions gives a bunch more flexibility. I'd rather see us move away from ArgumentChecker to Preconditions, but that's definitely for another ticket :)

Why don't you go ahead and put in a ticket for that, Josh. I think there's a lot we can offload onto Guava and this is one that I wasn't aware of that would be worthwhile.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16224/#review30279
-----------------------------------------------------------
On Dec. 16, 2013, 3:47 p.m., Bill Havanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16224/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2013, 3:47 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-1986
>     https://issues.apache.org/jira/browse/ACCUMULO-1986
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Integrity checks for constructing Key and Mutation objects from Thrift.
>
> To make adding unit tests easier, I updated KeyTest and MutationTest to use JUnit 4 annotations. That drove the need to rename an assertEquals method in MutationTest, hence all the changes to "checkCU" there.
>
> See the JIRA ticket for a list of classes that I evaluated for these same sort of integrity checks.
>
>
> Diffs
> -----
>
>   src/core/src/main/java/org/apache/accumulo/core/data/Key.java cfb0b5c
>   src/core/src/main/java/org/apache/accumulo/core/data/Mutation.java 3979da9
>   src/core/src/test/java/org/apache/accumulo/core/data/KeyTest.java 9a7f0d7
>   src/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java 38ddcad
>
> Diff: https://reviews.apache.org/r/16224/diff/
>
>
> Testing
> -------
>
> Unit tests and functional tests pass.
>
>
> Thanks,
>
> Bill Havanki
>
>