> On July 3, 2013, 3:58 p.m., bharat kumar wrote: > > server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 3054 > > <https://reviews.apache.org/r/12251/diff/1/?file=318002#file318002line3054> > > > > removed the sql exception > > Alena Prokharchyk wrote: > Bharat, I still see SQLException being caught: > > } catch (SQLException e) { > 3090 > throw new CloudRuntimeException(e.getMessage()); > 3104 > } > 3091 > } > > Can you please remove this try/catch as you are not throwing the > exception any longer? > > bharat kumar wrote: > The function deletePublicIPRangeExceptAliasIP used in > handleIpAliasDeletion throws the sql exception.
Well, you should remove it. This code: @Override public boolean deletePublicIPRangeExceptAliasIP(long vlanDbId, String aliasIp) throws SQLException { Transaction txn = Transaction.currentTxn(); String deleteSql = "DELETE FROM `cloud`.`user_ip_address` WHERE vlan_db_id = ? and public_ip_address!=?"; txn.start(); PreparedStatement stmt = txn.prepareAutoCloseStatement(deleteSql); stmt.setLong(1, vlanDbId); stmt.setString(2, aliasIp); stmt.executeUpdate(); txn.commit(); return true; } uses raw mysql update. We should never do that. What if vlan_db_id/public_ip_address are renamed or gone in the future? It won't be caught during compilation time. Please rewrite this code the following way: * use SearchBuilder or SearchCriteria to identify the rows you need to remove. Remove the raw mysql statement. * remove PreparedStatement update. * remove SQLException from the throws clause and from the underlying try/catch blocks. - Alena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12251/#review22722 ----------------------------------------------------------- On July 4, 2013, 2:09 a.m., bharat kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12251/ > ----------------------------------------------------------- > > (Updated July 4, 2013, 2:09 a.m.) > > > Review request for cloudstack, Alena Prokharchyk, Abhinandan Prateek, and > Sheng Yang. > > > Repository: cloudstack-git > > > Description > ------- > > Incorporating the review comments given by Alena and Sheng. > > > Diffs > ----- > > api/src/com/cloud/network/Network.java a06208b > api/src/com/cloud/network/NetworkService.java 405cecd > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 27c1a51 > server/src/com/cloud/network/NetworkManagerImpl.java 708c03d > server/src/com/cloud/network/NetworkServiceImpl.java 8cdcfde > server/src/com/cloud/network/element/VirtualRouterElement.java 1916678 > server/test/com/cloud/network/MockNetworkManagerImpl.java 077395f > server/test/com/cloud/vpc/MockNetworkManagerImpl.java b609022 > > Diff: https://reviews.apache.org/r/12251/diff/ > > > Testing > ------- > > > Thanks, > > bharat kumar > >