adutra commented on code in PR #3442:
URL: https://github.com/apache/polaris/pull/3442#discussion_r2703935487
##########
runtime/service/src/main/java/org/apache/polaris/service/events/listeners/PolarisPersistenceEventListener.java:
##########
@@ -30,17 +31,16 @@
import org.apache.polaris.core.auth.PolarisPrincipal;
import org.apache.polaris.service.events.EventAttributes;
import org.apache.polaris.service.events.PolarisEvent;
+import org.apache.polaris.service.events.PolarisEventType;
public abstract class PolarisPersistenceEventListener implements
PolarisEventListener {
@Override
- public void onEvent(PolarisEvent event) {
- switch (event.type()) {
- case AFTER_CREATE_TABLE -> handleAfterCreateTable(event);
- case AFTER_CREATE_CATALOG -> handleAfterCreateCatalog(event);
- default -> {
- // Other events not handled by this listener
- }
+ public void onEvent(PolarisEventType type, Supplier<PolarisEvent>
eventSupplier) {
+ switch (type) {
+ case AFTER_CREATE_TABLE -> handleAfterCreateTable(eventSupplier.get());
+ case AFTER_CREATE_CATALOG ->
handleAfterCreateCatalog(eventSupplier.get());
+ default -> {}
Review Comment:
Maybe keep the comment?
##########
runtime/service/src/main/java/org/apache/polaris/service/events/listeners/PolarisEventListener.java:
##########
@@ -18,14 +18,16 @@
*/
package org.apache.polaris.service.events.listeners;
+import java.util.function.Supplier;
import org.apache.polaris.service.events.PolarisEvent;
+import org.apache.polaris.service.events.PolarisEventType;
/**
- * Event listener that responds to notable moments during Polaris's execution.
Implementations can
- * filter events by checking {@link PolarisEvent#type()} or by querying
attributes with {@link
- * PolarisEvent#hasAttribute} and {@link PolarisEvent#attribute}.
+ * Event listener that responds to notable moments during Polaris's execution.
Implementations
+ * should check the event type and only call {@code eventSupplier.get()} for
types they handle,
+ * avoiding unnecessary object allocation.
*/
public interface PolarisEventListener {
- default void onEvent(PolarisEvent event) {}
+ default void onEvent(PolarisEventType type, Supplier<PolarisEvent>
eventSupplier) {}
Review Comment:
Unfortunately this is not binary-compatible. We would need to keep the old
method around. e.g.
```java
default void onEvent(PolarisEventType type, Supplier<PolarisEvent> supplier)
{ onEvent(supplier.get()); }
@Deprecated
default void onEvent(PolarisEventType type, Supplier<PolarisEvent>
eventSupplier) {}
```
--
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]
