[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4642: Display proper error message while deleting networks

[GitBox <gi...@apache.org>]

harikrishna-patnala commented on a change in pull request #4642:
URL: https://github.com/apache/cloudstack/pull/4642#discussion_r569109914



##########
File path: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -2882,8 +2882,9 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con
 
         for (final UserVmVO vm : userVms) {
             if (!(vm.getState() == VirtualMachine.State.Expunging && vm.getRemoved() != null)) {
-                s_logger.warn("Can't delete the network, not all user vms are expunged. Vm " + vm + " is in " + vm.getState() + " state");
-                return false;
+                final String message = "Can't delete the network, not all user vms are expunged. Vm " + vm + " is in " + vm.getState() + " state";
+                s_logger.warn(message);
+                throw new InvalidParameterValueException(message);

Review comment:
       Throwing exceptions will cause other issues here, as already mentioned by Daan, other components are handling the boolean response either by logging error message or cleaning up some other resources upon error (AccountManagerImpl).




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

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