----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20697/#review41603 -----------------------------------------------------------
samza-core/src/test/scala/org/apache/samza/system/TestSystemConsumers.scala <https://reviews.apache.org/r/20697/#comment75131> It's dangerous to catch Throwable -- this should probably be catching Exception. (See https://issues.apache.org/jira/browse/SAMZA-178) Or use the "intercept" feature of ScalaTest, as mentioned in another comment. Also, nit: looks like you're using tab characters here for indentation. I think we're using spaces everywhere else, and different people have different settings for the width of tab characters (2, 4, 8, ...) in their editors, so I'd prefer if we could stick with spaces everywhere. - Martin Kleppmann On April 25, 2014, 9:27 p.m., Yan Fang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20697/ > ----------------------------------------------------------- > > (Updated April 25, 2014, 9:27 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > Fail early when a consumer is misconfigured. > > 1. wraped the consumer register with try and catch > 2. added SystemConsumerException extends SamzaException > 3. added test for checking throwing the correct exception > > > Diffs > ----- > > samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala > 7624aef > samza-core/src/test/scala/org/apache/samza/system/TestSystemConsumers.scala > e1b211d > > Diff: https://reviews.apache.org/r/20697/diff/ > > > Testing > ------- > > > Thanks, > > Yan Fang > >
