----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25260/#review52094 -----------------------------------------------------------
server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java <https://reviews.apache.org/r/25260/#comment90830> A comment about why we throw a RTE for ConstraintViolationException but none of the others would be nice (in other words, reference the reason these changes are being done). test/src/test/java/org/apache/accumulo/test/Accumulo3096IT.java <https://reviews.apache.org/r/25260/#comment90831> It'd be much nicer to have a test name that is more meaningful than a ticket. Having the ticket referenced in a comment is useful if I want to find out more, but at first glance it doesn't tell me anything about what it's testing. test/src/test/java/org/apache/accumulo/test/Accumulo3096IT.java <https://reviews.apache.org/r/25260/#comment90832> Message for the failure would be nice. test/src/test/java/org/apache/accumulo/test/Accumulo3096IT.java <https://reviews.apache.org/r/25260/#comment90833> Would be better as an Assert.equals() call with a message as to why we expect the CVE as the cause. - Josh Elser On Sept. 2, 2014, 5:45 p.m., kturner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25260/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2014, 5:45 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-3096 > https://issues.apache.org/jira/browse/ACCUMULO-3096 > > > Repository: accumulo > > > Description > ------- > > Throw exception on metadata constraint violation, instead of retrying > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java d6762e7 > > server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java > 463ca57 > test/src/test/java/org/apache/accumulo/test/Accumulo3096IT.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25260/diff/ > > > Testing > ------- > > Ran mvn package w/o incident . Currently running mvn verify. > > > Thanks, > > kturner > >
