nvazquez commented on a change in pull request #5329:
URL: https://github.com/apache/cloudstack/pull/5329#discussion_r690864996



##########
File path: 
plugins/metrics/src/main/java/org/apache/cloudstack/response/HostMetricsResponse.java
##########
@@ -212,4 +216,13 @@ public void setMemoryAllocatedDisableThreshold(final Long 
memAllocated, final Lo
         }
     }
 
+    private Double parseCPU(String cpu) {

Review comment:
       Can we add some unit tests for it? 

##########
File path: 
plugins/metrics/src/main/java/org/apache/cloudstack/response/HostMetricsResponse.java
##########
@@ -212,4 +216,13 @@ public void setMemoryAllocatedDisableThreshold(final Long 
memAllocated, final Lo
         }
     }
 
+    private Double parseCPU(String cpu) {
+        DecimalFormat decimalFormat = new DecimalFormat("#.##");
+        try {
+            return decimalFormat.parse(cpu).doubleValue();
+        } catch (ParseException e) {
+            throw new CloudRuntimeException(e);

Review comment:
       What about logging the exception and handling the exception? Either here 
or in the usage of this function. For example, the character in the PR 
description can cause the API to still fail:
   ````
   "cpuallocated": "∞%",
   "cpuallocatedpercentage": "∞%",
   "cpuallocatedvalue": 500,
   "cpuallocatedwithoverprovisioning": "∞%",
   ````




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to