keith-turner commented on code in PR #5532: URL: https://github.com/apache/accumulo/pull/5532#discussion_r2085590655
########## core/src/main/java/org/apache/accumulo/core/spi/compaction/CompactionPlanner.java: ########## @@ -94,6 +95,12 @@ public interface PlanningParameters { */ TableId getTableId(); + /** + * @return the tablet for which a compaction is being planned + * @since 2.1.4 + */ + TabletId getTabletId(); Review Comment: I think leaving these methods unimplemented w/o a default impl is best because that is the most straightforward way for a developer to know they need to do something if they implemented this interface. If a user were to implement this interface it would most likely be for testing and it would be easy for a developer to deai with test code not implementing a method. If we want to add default method I think it would be best to throw unsupported op exception instead of returning null. Returning null has the potential to cause runtime exceptions that are far from the method that returned null, making it harder to track down. The unsupported op exception makes the issue easy to track down. We have returned unsupported op exception in other places in the past and that turned out to be bad, but that was a bit different. In this case its a completely new method that no 2.1.3 code could have called, we are not all of a sudden throwing that exception for an existing method. -- 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: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org