andrew4699 commented on code in PR #922: URL: https://github.com/apache/polaris/pull/922#discussion_r2003840573
########## service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java: ########## @@ -1229,11 +1243,14 @@ public void doRefresh() { Set.of(PolarisStorageActions.READ)); return TableMetadataParser.read(fileIO, metadataLocation); }); + polarisEventListener.onAfterTableRefreshed(new AfterTableRefreshedEvent(tableIdentifier)); Review Comment: Again, "because it's possible" is not the justification. There are use cases for each event implemented here, and there should be a demonstrable one for events added in the future. The design doc says this: > > The intent is for the list of events to grow with Polaris and for adding an event to feel like a relatively lightweight change, compared to refactoring an entire component into one/many swappable interfaces. Not everything should be an event though; there should be at least 3 criteria for adding a new event: > - Difficult to achieve through other means (configuration/dependency injection/etc) > - A use case can be demonstrated, although it may not be obviously useful to all users of the OSS project > - Cannot be folded into existing event listeners All 3 criteria are true for events in this PR. -- 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: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org