JoaoJandre commented on code in PR #9888:
URL: https://github.com/apache/cloudstack/pull/9888#discussion_r1856549459


##########
engine/schema/src/main/java/com/cloud/usage/dao/UsageVpcDaoImpl.java:
##########
@@ -64,11 +63,10 @@ public void remove(long vpcId, Date removed) {
             SearchCriteria<UsageVpcVO> sc = this.createSearchCriteria();
             sc.addAnd("vpcId", SearchCriteria.Op.EQ, vpcId);
             sc.addAnd("removed", SearchCriteria.Op.NULL);
-            UsageVpcVO vo = findOneBy(sc);
-            if (vo != null) {
-                vo.setRemoved(removed);
-                vo.setState(Vpc.State.Inactive.name());
-                update(vo.getId(), vo);
+            List<UsageVpcVO> usageVpcVOs = listBy(sc);
+            for (UsageVpcVO entry : usageVpcVOs) {
+                entry.setRemoved(removed);
+                update(entry.getId(), entry);

Review Comment:
   Same about the state



##########
usage/src/main/java/com/cloud/usage/UsageManagerImpl.java:
##########
@@ -1544,7 +1544,7 @@ private void createVolumeHelperEvent(UsageEventVO event) {
             //For volumes which are 'attached' successfully, set the 'deleted' 
column in the usage_storage table,
             //so that the secondary storage should stop accounting and only 
primary will be accounted.
             SearchCriteria<UsageStorageVO> sc = 
_usageStorageDao.createSearchCriteria();
-            sc.addAnd("id", SearchCriteria.Op.EQ, volId);
+            sc.addAnd("entityId", SearchCriteria.Op.EQ, volId);

Review Comment:
   You could extract these strings to constants



##########
engine/schema/src/main/java/com/cloud/usage/dao/UsageNetworksDaoImpl.java:
##########
@@ -70,11 +69,10 @@ public void remove(long networkId, Date removed) {
             SearchCriteria<UsageNetworksVO> sc = this.createSearchCriteria();
             sc.addAnd("networkId", SearchCriteria.Op.EQ, networkId);
             sc.addAnd("removed", SearchCriteria.Op.NULL);
-            UsageNetworksVO vo = findOneBy(sc);
-            if (vo != null) {
-                vo.setRemoved(removed);
-                vo.setState(Network.State.Destroy.name());
-                update(vo.getId(), vo);
+            List<UsageNetworksVO> usageNetworksVOs = listBy(sc);
+            for (UsageNetworksVO entry : usageNetworksVOs) {
+                entry.setRemoved(removed);
+                update(entry.getId(), entry);

Review Comment:
   Shouldn't the state be set as well?



-- 
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-unsubscr...@cloudstack.apache.org

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

Reply via email to