> On July 23, 2015, 12:37 a.m., Edward Ribeiro wrote: > > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala, line 269 > > <https://reviews.apache.org/r/36670/diff/1/?file=1018343#file1018343line269> > > > > I disagree because we are issuing two `deleteTopic` operations, the > > first one should succeed and the second one should fail with an specific > > exception, but in spite of this the topic should have been deleted by the > > first call, even if the fail is called. But I am okay with removing the > > finally block and have it after the catch.
You changed it, so it's more of an academic discussion, but here it goes. If an unexpected exception is thrown, then you don't know if the first `deleteTopic` succeeded (it could be thrown during that call) and the original exception would not be seen because another exception would be thrown during finally (shadowing it). It's generally a good idea to avoid throwing exceptions in `finally` if at all possible. - Ismael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36670/#review92688 ----------------------------------------------------------- On July 23, 2015, 1:23 a.m., Edward Ribeiro wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36670/ > ----------------------------------------------------------- > > (Updated July 23, 2015, 1:23 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2355 > https://issues.apache.org/jira/browse/KAFKA-2355 > > > Repository: kafka > > > Description > ------- > > KAFKA-2355 Add an unit test to validate the deletion of a partition marked as > deleted > > > Diffs > ----- > > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala > fa8ce259a2832ab86f9dda8c1d409b2c42d43ae9 > > Diff: https://reviews.apache.org/r/36670/diff/ > > > Testing > ------- > > > Thanks, > > Edward Ribeiro > >
