Jackie-Jiang commented on code in PR #15466:
URL: https://github.com/apache/pinot/pull/15466#discussion_r2032007475


##########
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java:
##########
@@ -1200,6 +1235,16 @@ private long convertPeriodToSeconds(String period) {
     return convertPeriodToUnit(period, TimeUnit.SECONDS);
   }
 
+  private boolean isValidPeriodWithLogging(String propertyKey, String 
periodStr) {
+    if (TimeUtils.isPeriodValid(periodStr)) {
+      return true;
+    } else {
+      LOGGER.error("Invalid time spec '{}' for config '{}'. Falling back to 
default config.", periodStr,

Review Comment:
   Logging an error is not very user friendly because a lot of people don't 
really check logs. We should store these errors in the service starter, and 
hook them up with the UI so that user knows these configs are not picked up



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to