shounakmk219 commented on code in PR #15118: URL: https://github.com/apache/pinot/pull/15118#discussion_r1977448235
########## pinot-minion/src/main/java/org/apache/pinot/minion/event/MinionProgressObserver.java: ########## @@ -62,14 +64,29 @@ public synchronized void notifyTaskStart(PinotTaskConfig pinotTaskConfig) { */ @Override public synchronized void notifyProgress(PinotTaskConfig pinotTaskConfig, @Nullable Object progress) { - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Update progress: {} for task: {}", progress, pinotTaskConfig.getTaskId()); - } + String progressMessage = null; + MinionTaskBaseObserverStats.StatusEntry statusEntry = null; _taskProgressStats.setCurrentState(MinionTaskState.IN_PROGRESS.name()); - addStatus(new MinionTaskBaseObserverStats.StatusEntry.Builder() - .withTs(System.currentTimeMillis()) - .withStatus((progress == null) ? "" : progress.toString()) - .build()); + if (progress instanceof MinionTaskBaseObserverStats.StatusEntry) { Review Comment: As the existing `notifyProgress` interface method accepts progress as Object itself, we are forced to use instance of. Task executors should be able to pass any of StatusEntry, ObserverStats or String based on what info it wants to track -- 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