[GitHub] [cloudstack] nvazquez commented on a change in pull request #5329: metrics: fix hostsmetricsresponse for zero cpu, locale

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

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: commits-unsubscribe@cloudstack.apache.org

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