> On March 26, 2015, 8:04 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java, > > line 180 > > <https://reviews.apache.org/r/32327/diff/1/?file=901649#file901649line180> > > > > Is the default value validated somewhere else?
The default value is not validated. In fact it might not be valid. In the case of DRILLBIT_EXCEPTION_INJECTIONS, you used the empty string (causes an ExpressionParsingException) as a quick check to not inject anything. I will change it to enforce a valid default value (and change the default for DRILLBIT_EXCEPTION_INJECTIONS). > On March 26, 2015, 8:04 p.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java, > > line 121 > > <https://reviews.apache.org/r/32327/diff/1/?file=901651#file901651line121> > > > > Is there any way to check if requestedPort was actually specified (if > > requestedAddress is non-null), and isn't just defaulting to zero? Sadly, no. Jackson assigns zero as a default value to int. > On March 26, 2015, 8:04 p.m., Chris Westin wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java, > > line 144 > > <https://reviews.apache.org/r/32327/diff/1/?file=901655#file901655line144> > > > > Do we know that there's only one drillbit running for this test, or > > should we just make the test injections here global? (1) We do not know. (2) This test just checks that DummyClass throws an unchecked exception in any bit context (happens to be the context of bits[0]). So no. - Sudheesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32327/#review77309 ----------------------------------------------------------- On March 20, 2015, 8:20 p.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32327/ > ----------------------------------------------------------- > > (Updated March 20, 2015, 8:20 p.m.) > > > Review request for drill and Chris Westin. > > > Repository: drill-git > > > Description > ------- > > [DRILL-2437](https://issues.apache.org/jira/browse/DRILL-2437): Moved > SimulatedExceptions to UserSession > > + [DRILL-2382](https://issues.apache.org/jira/browse/DRILL-2382): Added > address and port to InjectionOption to specify drillbit > + JSONStringValidator to TypeValidators > > > Diffs > ----- > > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java > cd0a0a2 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserSession.java > efb0cdf > > exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java > dbf3c74 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/TypeValidators.java > b9721cc > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java > 54bc351 > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java > 0292c08 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java > bfb6de8 > > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java > 9bc0552 > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java > bf93dee > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java > d0c0279 > > Diff: https://reviews.apache.org/r/32327/diff/ > > > Testing > ------- > > Added test case. > > Passes all other tests, and builds. > > > Thanks, > > Sudheesh Katkam > >
