kfaraz commented on code in PR #16065:
URL: https://github.com/apache/druid/pull/16065#discussion_r1515608613


##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java:
##########
@@ -1108,13 +1108,12 @@ private static InputFormat 
getInputFormat(IndexIngestionSpec ingestionSchema)
   }
 
   @Override
-  public void cleanUp(TaskToolbox toolbox, @Nullable TaskStatus taskStatus) 
throws Exception
+  protected boolean shouldCleanupTask()

Review Comment:
   Is this change really needed?
   Is `shouldCleanupTask()` meant to be used in implementations other than 
`IndexTask`?
   
   The javadoc of `Task.cleanup()` already calls out that this method is 
overridden only by implementations that need to perform any special cleanup. If 
not overridden, the default impl of `cleanup` is noop.
   Adding another `shouldCleanupTask()` method in the mix seems unnecessary, 
esp since it is used in just one place.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to