> On Aug. 15, 2015, 4:11 p.m., Jarek Cecho wrote: > > core/src/main/java/org/apache/sqoop/core/SqoopServer.java, lines 61-65 > > <https://reviews.apache.org/r/37474/diff/1/?file=1039990#file1039990line61> > > > > Since we moved to JDK7 now, what about having one catch clause for both > > cases? > > > > > > https://docs.oracle.com/javase/7/docs/technotes/guides/language/catch-multiple.html#multiple > > Colin Ma wrote: > From the FindBugs, the RuntimeException should be catched in theses code. > But the RuntimeException is a subclass of Exception, so the > catch-multiple doesn't work for this case. > I'll upate the patch to add log.error for RuntimeException.
I see, thanks for the expplanation Colin. I think that we've put the Exception there to simply "catch all" case. Since findbugs is complaining about it (and rightfully so), what about explicitly iterating what exceptions we're catching? RuntimeException will definitely be on the list, not sure what others. What do you think? - Jarek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37474/#review95519 ----------------------------------------------------------- On Aug. 18, 2015, 2:29 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37474/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2015, 2:29 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Fix smaller-ish warnings in core module > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/audit/AuditLoggerManager.java 728127b > core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java a16fceb > > core/src/main/java/org/apache/sqoop/core/PropertiesConfigurationProvider.java > f1c4820 > core/src/main/java/org/apache/sqoop/core/SqoopServer.java e78e4d9 > core/src/main/java/org/apache/sqoop/driver/JobManager.java d4e0655 > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java c09b77b > > core/src/main/java/org/apache/sqoop/repository/JdbcTransactionIsolation.java > 2b1c8ce > core/src/main/java/org/apache/sqoop/repository/Repository.java 8cbff99 > > Diff: https://reviews.apache.org/r/37474/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
