[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #7630: cleanup removal of physical network

["harikrishna-patnala (via GitHub)" <gi...@apache.org>]

harikrishna-patnala commented on code in PR #7630:
URL: https://github.com/apache/cloudstack/pull/7630#discussion_r1230496290


##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -4264,23 +4265,37 @@ public boolean deletePhysicalNetwork(final Long physicalNetworkId) {
         return Transaction.execute(new TransactionCallback<Boolean>() {
             @Override
             public Boolean doInTransaction(TransactionStatus status) {
-                // delete vlans for this zone
-                List<VlanVO> vlans = _vlanDao.listVlansByPhysicalNetworkId(physicalNetworkId);
-                for (VlanVO vlan : vlans) {
-                    _vlanDao.remove(vlan.getId());
-                }
-
-                // Delete networks
-                List<NetworkVO> networks = _networksDao.listByPhysicalNetwork(physicalNetworkId);
-                if (networks != null && !networks.isEmpty()) {
-                    for (NetworkVO network : networks) {
-                        _networksDao.remove(network.getId());
-                    }
-                }
+                disablePhysicalNetwork(physicalNetworkId, pNetwork);

Review Comment:
   Isn't better to reverse the order of these function calls? yes, these are in a transaction but better to be in the reverse flow.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org