> 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
>
>