You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "nandorKollar (via GitHub)" <gi...@apache.org> on 2026/03/26 13:53:17 UTC
[PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
nandorKollar opened a new pull request, #4064:
URL: https://github.com/apache/polaris/pull/4064
<!--
π Describe what changes you're proposing, especially breaking or user-facing changes.
π See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
This change introduces new configuration option to allow each listener to listen only a subset of each Polaris event. The change also includes an optimization: the PolarisEvent doesn't get created and it isn't dispatched if there's no even listener listening on the given event type.
The PR is based on #3442 and incorporates recommendations from @adutra .
## Checklist
- [ ] π‘οΈ Don't disclose security issues! (contact security@apache.org)
- [ ] π Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] π§ͺ Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] π‘ Added comments for complex logic
- [ ] π§Ύ Updated `CHANGELOG.md` (if needed)
- [ ] π Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3074522305
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
- Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes =
+ listenerConfiguration == null
Review Comment:
@adnanhemani : Do you mean we should ask the listener for the set of event types it needs to process?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3050769630
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ Arrays.stream(PolarisEventType.values())
+ .filter(e -> e.category() == PolarisEventType.Category.CATALOG)
+ .collect(Collectors.toSet());
+
+ static final CountDownLatch latch = new CountDownLatch(5 + 3 * CATALOG_EVENTS.size());
Review Comment:
This latch looks a bit fragile here imo (the latch size in particular looks a bit "magic"). Would it be possible to use Awaitility instead?
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ Arrays.stream(PolarisEventType.values())
+ .filter(e -> e.category() == PolarisEventType.Category.CATALOG)
+ .collect(Collectors.toSet());
+
+ static final CountDownLatch latch = new CountDownLatch(5 + 3 * CATALOG_EVENTS.size());
+
+ private abstract static class FilteringEventListener implements PolarisEventListener {
+ private final Predicate<PolarisEvent> predicate;
+
+ List<PolarisEvent> expectedEvents = new ArrayList<>();
Review Comment:
I would go with `CopyOnWriteArrayList` here β events can be delivered concurrently.
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ Arrays.stream(PolarisEventType.values())
+ .filter(e -> e.category() == PolarisEventType.Category.CATALOG)
+ .collect(Collectors.toSet());
+
+ static final CountDownLatch latch = new CountDownLatch(5 + 3 * CATALOG_EVENTS.size());
+
+ private abstract static class FilteringEventListener implements PolarisEventListener {
+ private final Predicate<PolarisEvent> predicate;
+
+ List<PolarisEvent> expectedEvents = new ArrayList<>();
+ List<PolarisEvent> unexpectedEvents = new ArrayList<>();
+
+ FilteringEventListener(Predicate<PolarisEvent> predicate) {
+ this.predicate = predicate;
+ }
+
+ @Override
+ public void onEvent(PolarisEvent event) {
+ if (predicate.test(event)) {
+ expectedEvents.add(event);
+ } else {
+ unexpectedEvents.add(event);
+ }
+ latch.countDown();
+ }
+ }
+
+ @Singleton
+ @Identifier("after-send-event-listener")
+ public static class AfterSendEventListener extends FilteringEventListener {
+ AfterSendEventListener() {
+ super(event -> event.type() == PolarisEventType.AFTER_SEND_NOTIFICATION);
+ }
+ }
+
+ @Singleton
+ @Identifier("before-send-event-listener")
+ public static class BeforeSendEventListener extends FilteringEventListener {
+ BeforeSendEventListener() {
+ super(event -> event.type() == PolarisEventType.BEFORE_SEND_NOTIFICATION);
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-all-listener")
+ public static class ConsumeAllEventListener implements PolarisEventListener {
+ List<PolarisEvent> consumedEvents = new ArrayList<>();
+
+ @Override
+ public void onEvent(PolarisEvent event) {
+ consumedEvents.add(event);
+ latch.countDown();
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-only-catalog-events-listener")
+ public static class ConsumeOnlyCatalogEventsListener extends FilteringEventListener {
+ ConsumeOnlyCatalogEventsListener() {
+ super(event -> event.type().category() == PolarisEventType.Category.CATALOG);
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-catalog-and-after-notification-events-listener")
+ public static class ConsumeCatalogAndNotificationEventsListener extends FilteringEventListener {
+ ConsumeCatalogAndNotificationEventsListener() {
+ super(
+ event ->
+ event.type().category() == PolarisEventType.Category.CATALOG
+ || event.type() == PolarisEventType.AFTER_SEND_NOTIFICATION);
+ }
+ }
+
+ public static class PolarisEventListenersTestProfile implements QuarkusTestProfile {
+ @Override
+ public Map<String, String> getConfigOverrides() {
+ return ImmutableMap.<String, String>builder()
+ .put(
+ "polaris.event-listener.types",
+ "after-send-event-listener,before-send-event-listener,consume-all-listener,consume-only-catalog-events-listener,consume-catalog-and-after-notification-events-listener")
+ .put(
+ "polaris.event-listener.after-send-event-listener.enabled-event-types",
+ "AFTER_SEND_NOTIFICATION")
+ .put(
+ "polaris.event-listener.before-send-event-listener.enabled-event-types",
+ "BEFORE_SEND_NOTIFICATION")
+ .put(
+ "polaris.event-listener.consume-only-catalog-events-listener.enabled-event-categories",
+ "CATALOG")
+ .put(
+ "polaris.event-listener.consume-catalog-and-after-notification-events-listener.enabled-event-categories",
+ "CATALOG")
+ .put(
+ "polaris.event-listener.consume-catalog-and-after-notification-events-listener.enabled-event-types",
+ "AFTER_SEND_NOTIFICATION")
+ .build();
+ }
+ }
+
+ @Inject PolarisEventDispatcher eventDispatcher;
+
+ @Inject
+ @Identifier("after-send-event-listener")
+ PolarisEventListener afterSendEventListener;
+
+ @Inject
+ @Identifier("before-send-event-listener")
+ PolarisEventListener beforeSendEventListener;
+
+ @Inject
+ @Identifier("consume-all-listener")
+ PolarisEventListener consumeAllEventListener;
+
+ @Inject
+ @Identifier("consume-only-catalog-events-listener")
+ PolarisEventListener consumeOnlyCatalogEventListener;
+
+ @Inject
+ @Identifier("consume-catalog-and-after-notification-events-listener")
+ PolarisEventListener consumeCatalogAndNotificationEventsListener;
+
+ @Test
+ public void testEventListenersGetNotified() throws InterruptedException {
Review Comment:
One of the tests we had in `PolarisEventListenersMultipleListenersTest` was testing that *two* listeners without any type filtering both receive all events. We don't have this specific test here. Not sure we absolutely must test this scenario, but I wanted to note the difference.
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -41,13 +42,35 @@ public class PolarisEventListeners {
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes = PolarisEventType.values();
+ if (listenerConfiguration != null) {
+ Set<PolarisEventType> enabledEventTypes = new HashSet<>();
+ if (listenerConfiguration.enabledEventCategories().isPresent()) {
+ for (var enabledEventCategory : listenerConfiguration.enabledEventCategories().get()) {
+ enabledEventTypes.addAll(PolarisEventType.typesOfCategory(enabledEventCategory));
+ }
+ }
+ if (listenerConfiguration.enabledEventTypes().isPresent()) {
+ enabledEventTypes.addAll(listenerConfiguration.enabledEventTypes().get());
+ }
+ supportedTypes = enabledEventTypes.toArray(PolarisEventType[]::new);
+ }
+ var listener = eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
Review Comment:
I suggest extracting the delivery logic to a method and surround with exception handling + debug logging, because handlers are not supposed to throw:
```java
Handler<Message<PolarisEvent>> handler =
message -> deliverEvent(message.body(), enabledEventListener, listener);
for (var polarisEventType : supportedTypes) {
eventsByType.set(polarisEventType.ordinal());
eventBus.localConsumer(POLARIS_EVENT_CHANNEL + "." + polarisEventType, handler);
}
}
}
private void deliverEvent(
PolarisEvent event, String listenerName, PolarisEventListener listener) {
LOGGER.debug("Delivering {} event to listener '{}' ({})", event.type(), listenerName, listener);
try {
listener.onEvent(event);
} catch (Exception e) {
LOGGER.error(
"Error while delivering {} event to listener '{}' ({})",
event.type(),
listenerName,
listener,
e);
} finally {
LOGGER.debug(
"Delivered {} event to listener '{}' ({})", event.type(), listenerName, listener);
}
}
```
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListenerConfiguration.java:
##########
@@ -41,4 +44,25 @@ public interface PolarisEventListenerConfiguration {
* PolarisEventListener} identifier.
*/
Optional<Set<String>> types();
+
+ /** Configuration of each event listener type. */
+ @WithParentName
+ Map<String, ListenerConfiguration> listenerConfig();
+
+ interface ListenerConfiguration {
+ /**
+ * Comma separated list of enabled event types. This event listener will only receive events of
+ * the selected types. If both the event types and event category configs are set, the listener
Review Comment:
I think we should also mention what the defaults are:
- If no listener configuration exists, all events are enabled.
- If a listener configuration exists, events are disabled by default (unless explicitlty listed in either `enabled-event-types` or `enabled-event-categories`).
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -41,13 +42,35 @@ public class PolarisEventListeners {
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes = PolarisEventType.values();
+ if (listenerConfiguration != null) {
+ Set<PolarisEventType> enabledEventTypes = new HashSet<>();
+ if (listenerConfiguration.enabledEventCategories().isPresent()) {
+ for (var enabledEventCategory : listenerConfiguration.enabledEventCategories().get()) {
+ enabledEventTypes.addAll(PolarisEventType.typesOfCategory(enabledEventCategory));
+ }
+ }
+ if (listenerConfiguration.enabledEventTypes().isPresent()) {
+ enabledEventTypes.addAll(listenerConfiguration.enabledEventTypes().get());
+ }
+ supportedTypes = enabledEventTypes.toArray(PolarisEventType[]::new);
+ }
Review Comment:
Minor suggestion:
```suggestion
Set<PolarisEventType> supportedTypes;
if (listenerConfiguration != null) {
supportedTypes = EnumSet.noneOf(PolarisEventType.class);
if (listenerConfiguration.enabledEventCategories().isPresent()) {
for (var enabledEventCategory : listenerConfiguration.enabledEventCategories().get()) {
supportedTypes.addAll(PolarisEventType.typesOfCategory(enabledEventCategory));
}
}
if (listenerConfiguration.enabledEventTypes().isPresent()) {
supportedTypes.addAll(listenerConfiguration.enabledEventTypes().get());
}
} else {
// No configuration: enable all event types
supportedTypes = EnumSet.allOf(PolarisEventType.class);
}
```
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4197096222
> Thanks β I walked through the latest revision and I donβt see an obvious correctness blocker after the event-bus address fix.
>
> The main thing Iβd still like to see is focused regression coverage for the new behavior introduced here:
>
> * a listener configured with enabledEventTypes only receives that subset;
> * when no listener is registered for a type, we really skip the PolarisEvent creation / dispatch path;
> * the deprecated polaris.event-listener.type fallback still behaves as expected alongside the new multi-listener config.
>
> This PR is changing both config semantics and the dispatch fast path, so having those cases pinned down in tests would make future refactors a lot safer.
Thanks @flyingImer for the review. I added some more test coverage, would you mind have a look at the latest version, and share your thoughts?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3074449396
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
Review Comment:
Yes, good idea.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3030837702
##########
site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_event_listener.md:
##########
@@ -25,5 +25,6 @@ build:
| Property | Default Value | Type | Description |
|----------|---------------|------|-------------|
-| `polaris.event-listener.type` | | `string` | The type of the event listener to use. Must be a registered (`PolarisEventListener`) identifier. <br><br>_Deprecated_ since 1.5.0, use 'polaris.event-listener.types' instead |
+| `polaris.event-listener.type` | | `string` | The type of the event listener to use. Must be a registered (`PolarisEventListener`) identifier. <br><br>_Deprecated_ since 1.5.0, use 'polaris.event-listener.types' instead, if both are set, then polaris.event-listener.types is prioritized |
| `polaris.event-listener.types` | | `list of string` | Comma separated list of event listers, each item must be a registered (`PolarisEventListener`) identifier. |
+| `polaris.event-listener.`_`<name>`_`.enabled-event-types` | | `list of BEFORE_CREATE_CATALOG, AFTER_CREATE_CATALOG, BEFORE_DELETE_CATALOG, AFTER_DELETE_CATALOG, BEFORE_GET_CATALOG, AFTER_GET_CATALOG, BEFORE_UPDATE_CATALOG, AFTER_UPDATE_CATALOG, BEFORE_LIST_CATALOGS, AFTER_LIST_CATALOGS, BEFORE_CREATE_CATALOG_ROLE, AFTER_CREATE_CATALOG_ROLE, BEFORE_DELETE_CATALOG_ROLE, AFTER_DELETE_CATALOG_ROLE, BEFORE_GET_CATALOG_ROLE, AFTER_GET_CATALOG_ROLE, BEFORE_UPDATE_CATALOG_ROLE, AFTER_UPDATE_CATALOG_ROLE, BEFORE_LIST_CATALOG_ROLES, AFTER_LIST_CATALOG_ROLES, BEFORE_ADD_GRANT_TO_CATALOG_ROLE, AFTER_ADD_GRANT_TO_CATALOG_ROLE, BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE, AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE, BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE, AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE, BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE, AFTER_LIST_GRANTS_FOR_CATALOG_ROLE, BEFORE_CREATE_PRINCIPAL, AFTER_CREATE_PRINCIPAL, BEFORE_DELETE_PRINCIPAL, AFTER_DELETE_PRINCIPAL, BEFORE_GET_PRIN
CIPAL, AFTER_GET_PRINCIPAL, BEFORE_UPDATE_PRINCIPAL, AFTER_UPDATE_PRINCIPAL, BEFORE_ROTATE_CREDENTIALS, AFTER_ROTATE_CREDENTIALS, BEFORE_RESET_CREDENTIALS, AFTER_RESET_CREDENTIALS, BEFORE_LIST_PRINCIPALS, AFTER_LIST_PRINCIPALS, BEFORE_ASSIGN_PRINCIPAL_ROLE, AFTER_ASSIGN_PRINCIPAL_ROLE, BEFORE_REVOKE_PRINCIPAL_ROLE, AFTER_REVOKE_PRINCIPAL_ROLE, BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES, AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES, BEFORE_CREATE_PRINCIPAL_ROLE, AFTER_CREATE_PRINCIPAL_ROLE, BEFORE_DELETE_PRINCIPAL_ROLE, AFTER_DELETE_PRINCIPAL_ROLE, BEFORE_GET_PRINCIPAL_ROLE, AFTER_GET_PRINCIPAL_ROLE, BEFORE_UPDATE_PRINCIPAL_ROLE, AFTER_UPDATE_PRINCIPAL_ROLE, BEFORE_LIST_PRINCIPAL_ROLES, AFTER_LIST_PRINCIPAL_ROLES, BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE, AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE, BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE, AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE, BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE, AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE,
BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE, AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE, BEFORE_CREATE_NAMESPACE, AFTER_CREATE_NAMESPACE, BEFORE_LIST_NAMESPACES, AFTER_LIST_NAMESPACES, BEFORE_LOAD_NAMESPACE_METADATA, AFTER_LOAD_NAMESPACE_METADATA, BEFORE_CHECK_EXISTS_NAMESPACE, AFTER_CHECK_EXISTS_NAMESPACE, BEFORE_DROP_NAMESPACE, AFTER_DROP_NAMESPACE, BEFORE_UPDATE_NAMESPACE_PROPERTIES, AFTER_UPDATE_NAMESPACE_PROPERTIES, BEFORE_CREATE_TABLE, AFTER_CREATE_TABLE, BEFORE_LIST_TABLES, AFTER_LIST_TABLES, BEFORE_LOAD_TABLE, AFTER_LOAD_TABLE, BEFORE_CHECK_EXISTS_TABLE, AFTER_CHECK_EXISTS_TABLE, BEFORE_DROP_TABLE, AFTER_DROP_TABLE, BEFORE_REGISTER_TABLE, AFTER_REGISTER_TABLE, BEFORE_RENAME_TABLE, AFTER_RENAME_TABLE, BEFORE_UPDATE_TABLE, AFTER_UPDATE_TABLE, BEFORE_REFRESH_TABLE, AFTER_REFRESH_TABLE, BEFORE_CREATE_VIEW, AFTER_CREATE_VIEW, BEFORE_LIST_VIEWS, AFTER_LIST_VIEWS, BEFORE_LOAD_VIEW, AFTER_LOAD_VIEW, BEFORE_CHECK_EXISTS_VIEW, AFTER_CHECK_EXISTS_VIEW, BEFORE_DROP_VIEW, AFTER_DROP_VI
EW, BEFORE_RENAME_VIEW, AFTER_RENAME_VIEW, BEFORE_REPLACE_VIEW, AFTER_REPLACE_VIEW, BEFORE_REFRESH_VIEW, AFTER_REFRESH_VIEW, BEFORE_LOAD_CREDENTIALS, AFTER_LOAD_CREDENTIALS, BEFORE_COMMIT_TRANSACTION, AFTER_COMMIT_TRANSACTION, BEFORE_SEND_NOTIFICATION, AFTER_SEND_NOTIFICATION, BEFORE_GET_CONFIG, AFTER_GET_CONFIG, BEFORE_CREATE_POLICY, AFTER_CREATE_POLICY, BEFORE_LIST_POLICIES, AFTER_LIST_POLICIES, BEFORE_LOAD_POLICY, AFTER_LOAD_POLICY, BEFORE_UPDATE_POLICY, AFTER_UPDATE_POLICY, BEFORE_DROP_POLICY, AFTER_DROP_POLICY, BEFORE_ATTACH_POLICY, AFTER_ATTACH_POLICY, BEFORE_DETACH_POLICY, AFTER_DETACH_POLICY, BEFORE_GET_APPLICABLE_POLICIES, AFTER_GET_APPLICABLE_POLICIES, BEFORE_CREATE_GENERIC_TABLE, AFTER_CREATE_GENERIC_TABLE, BEFORE_DROP_GENERIC_TABLE, AFTER_DROP_GENERIC_TABLE, BEFORE_LIST_GENERIC_TABLES, AFTER_LIST_GENERIC_TABLES, BEFORE_LOAD_GENERIC_TABLE, AFTER_LOAD_GENERIC_TABLE, BEFORE_ATTEMPT_TASK, AFTER_ATTEMPT_TASK, BEFORE_LIMIT_REQUEST_RATE` | Comma separated list of enabled event
types. This event listener will only receive events of the selected types. |
Review Comment:
+1
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3031727630
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
nit: We have a bunch of boilerplate here around create/delete/etc.
Maybe it would be cleaner to a helper like `dispatchIfListening(Supplier<PolarisEvent>)` or similar to simplify the maintenance.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3067902236
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
Review Comment:
IMO an EnumSet would be much more clear here.
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListenerConfiguration.java:
##########
Review Comment:
nit: "listeners"
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
- Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes =
+ listenerConfiguration == null
Review Comment:
This is a bit brittle logic. Say an Event Listener requires some specific configuration (unrelated to event types accepted) but the user does not want to explicitly define all of the PolarisEventTypes. What happens then?
IMO, we should ensure that we check for "enabledEventTypes" not being present at all.
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java:
##########
@@ -19,189 +19,223 @@
package org.apache.polaris.service.events;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
public enum PolarisEventType {
// Catalog Events
- BEFORE_CREATE_CATALOG(100),
- AFTER_CREATE_CATALOG(101),
- BEFORE_DELETE_CATALOG(102),
- AFTER_DELETE_CATALOG(103),
- BEFORE_GET_CATALOG(104),
- AFTER_GET_CATALOG(105),
- BEFORE_UPDATE_CATALOG(106),
- AFTER_UPDATE_CATALOG(107),
- BEFORE_LIST_CATALOGS(108),
- AFTER_LIST_CATALOGS(109),
+ BEFORE_CREATE_CATALOG(100, Category.CATALOG),
+ AFTER_CREATE_CATALOG(101, Category.CATALOG),
+ BEFORE_DELETE_CATALOG(102, Category.CATALOG),
+ AFTER_DELETE_CATALOG(103, Category.CATALOG),
+ BEFORE_GET_CATALOG(104, Category.CATALOG),
+ AFTER_GET_CATALOG(105, Category.CATALOG),
+ BEFORE_UPDATE_CATALOG(106, Category.CATALOG),
+ AFTER_UPDATE_CATALOG(107, Category.CATALOG),
+ BEFORE_LIST_CATALOGS(108, Category.CATALOG),
+ AFTER_LIST_CATALOGS(109, Category.CATALOG),
// Catalog Role Events
- BEFORE_CREATE_CATALOG_ROLE(200),
- AFTER_CREATE_CATALOG_ROLE(201),
- BEFORE_DELETE_CATALOG_ROLE(202),
- AFTER_DELETE_CATALOG_ROLE(203),
- BEFORE_GET_CATALOG_ROLE(204),
- AFTER_GET_CATALOG_ROLE(205),
- BEFORE_UPDATE_CATALOG_ROLE(206),
- AFTER_UPDATE_CATALOG_ROLE(207),
- BEFORE_LIST_CATALOG_ROLES(208),
- AFTER_LIST_CATALOG_ROLES(209),
- BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210),
- AFTER_ADD_GRANT_TO_CATALOG_ROLE(211),
- BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212),
- AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213),
- BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214),
- AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215),
- BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216),
- AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217),
+ BEFORE_CREATE_CATALOG_ROLE(200, Category.CATALOG_ROLE),
+ AFTER_CREATE_CATALOG_ROLE(201, Category.CATALOG_ROLE),
+ BEFORE_DELETE_CATALOG_ROLE(202, Category.CATALOG_ROLE),
+ AFTER_DELETE_CATALOG_ROLE(203, Category.CATALOG_ROLE),
+ BEFORE_GET_CATALOG_ROLE(204, Category.CATALOG_ROLE),
+ AFTER_GET_CATALOG_ROLE(205, Category.CATALOG_ROLE),
+ BEFORE_UPDATE_CATALOG_ROLE(206, Category.CATALOG_ROLE),
+ AFTER_UPDATE_CATALOG_ROLE(207, Category.CATALOG_ROLE),
+ BEFORE_LIST_CATALOG_ROLES(208, Category.CATALOG_ROLE),
+ AFTER_LIST_CATALOG_ROLES(209, Category.CATALOG_ROLE),
+ BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210, Category.CATALOG_ROLE),
+ AFTER_ADD_GRANT_TO_CATALOG_ROLE(211, Category.CATALOG_ROLE),
+ BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212, Category.CATALOG_ROLE),
+ AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213, Category.CATALOG_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214, Category.CATALOG_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215, Category.CATALOG_ROLE),
+ BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216, Category.CATALOG_ROLE),
+ AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217, Category.CATALOG_ROLE),
// Principal Events
- BEFORE_CREATE_PRINCIPAL(300),
- AFTER_CREATE_PRINCIPAL(301),
- BEFORE_DELETE_PRINCIPAL(302),
- AFTER_DELETE_PRINCIPAL(303),
- BEFORE_GET_PRINCIPAL(304),
- AFTER_GET_PRINCIPAL(305),
- BEFORE_UPDATE_PRINCIPAL(306),
- AFTER_UPDATE_PRINCIPAL(307),
- BEFORE_ROTATE_CREDENTIALS(308),
- AFTER_ROTATE_CREDENTIALS(309),
- BEFORE_RESET_CREDENTIALS(310),
- AFTER_RESET_CREDENTIALS(311),
- BEFORE_LIST_PRINCIPALS(312),
- AFTER_LIST_PRINCIPALS(313),
- BEFORE_ASSIGN_PRINCIPAL_ROLE(314),
- AFTER_ASSIGN_PRINCIPAL_ROLE(315),
- BEFORE_REVOKE_PRINCIPAL_ROLE(316),
- AFTER_REVOKE_PRINCIPAL_ROLE(317),
- BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318),
- AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319),
+ BEFORE_CREATE_PRINCIPAL(300, Category.PRINCIPAL),
+ AFTER_CREATE_PRINCIPAL(301, Category.PRINCIPAL),
+ BEFORE_DELETE_PRINCIPAL(302, Category.PRINCIPAL),
+ AFTER_DELETE_PRINCIPAL(303, Category.PRINCIPAL),
+ BEFORE_GET_PRINCIPAL(304, Category.PRINCIPAL),
+ AFTER_GET_PRINCIPAL(305, Category.PRINCIPAL),
+ BEFORE_UPDATE_PRINCIPAL(306, Category.PRINCIPAL),
+ AFTER_UPDATE_PRINCIPAL(307, Category.PRINCIPAL),
+ BEFORE_ROTATE_CREDENTIALS(308, Category.PRINCIPAL),
+ AFTER_ROTATE_CREDENTIALS(309, Category.PRINCIPAL),
+ BEFORE_RESET_CREDENTIALS(310, Category.PRINCIPAL),
+ AFTER_RESET_CREDENTIALS(311, Category.PRINCIPAL),
+ BEFORE_LIST_PRINCIPALS(312, Category.PRINCIPAL),
+ AFTER_LIST_PRINCIPALS(313, Category.PRINCIPAL),
+ BEFORE_ASSIGN_PRINCIPAL_ROLE(314, Category.PRINCIPAL),
+ AFTER_ASSIGN_PRINCIPAL_ROLE(315, Category.PRINCIPAL),
+ BEFORE_REVOKE_PRINCIPAL_ROLE(316, Category.PRINCIPAL),
+ AFTER_REVOKE_PRINCIPAL_ROLE(317, Category.PRINCIPAL),
+ BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318, Category.PRINCIPAL),
+ AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319, Category.PRINCIPAL),
// Principal Role Events
- BEFORE_CREATE_PRINCIPAL_ROLE(400),
- AFTER_CREATE_PRINCIPAL_ROLE(401),
- BEFORE_DELETE_PRINCIPAL_ROLE(402),
- AFTER_DELETE_PRINCIPAL_ROLE(403),
- BEFORE_GET_PRINCIPAL_ROLE(404),
- AFTER_GET_PRINCIPAL_ROLE(405),
- BEFORE_UPDATE_PRINCIPAL_ROLE(406),
- AFTER_UPDATE_PRINCIPAL_ROLE(407),
- BEFORE_LIST_PRINCIPAL_ROLES(408),
- AFTER_LIST_PRINCIPAL_ROLES(409),
- BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410),
- AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411),
- BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412),
- AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413),
- BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414),
- AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415),
- BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416),
- AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417),
+ BEFORE_CREATE_PRINCIPAL_ROLE(400, Category.PRINCIPAL_ROLE),
+ AFTER_CREATE_PRINCIPAL_ROLE(401, Category.PRINCIPAL_ROLE),
+ BEFORE_DELETE_PRINCIPAL_ROLE(402, Category.PRINCIPAL_ROLE),
+ AFTER_DELETE_PRINCIPAL_ROLE(403, Category.PRINCIPAL_ROLE),
+ BEFORE_GET_PRINCIPAL_ROLE(404, Category.PRINCIPAL_ROLE),
+ AFTER_GET_PRINCIPAL_ROLE(405, Category.PRINCIPAL_ROLE),
+ BEFORE_UPDATE_PRINCIPAL_ROLE(406, Category.PRINCIPAL_ROLE),
+ AFTER_UPDATE_PRINCIPAL_ROLE(407, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_PRINCIPAL_ROLES(408, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_PRINCIPAL_ROLES(409, Category.PRINCIPAL_ROLE),
+ BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410, Category.PRINCIPAL_ROLE),
+ AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411, Category.PRINCIPAL_ROLE),
+ BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412, Category.PRINCIPAL_ROLE),
+ AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417, Category.PRINCIPAL_ROLE),
// Namespace Events
- BEFORE_CREATE_NAMESPACE(500),
- AFTER_CREATE_NAMESPACE(501),
- BEFORE_LIST_NAMESPACES(502),
- AFTER_LIST_NAMESPACES(503),
- BEFORE_LOAD_NAMESPACE_METADATA(504),
- AFTER_LOAD_NAMESPACE_METADATA(505),
- BEFORE_CHECK_EXISTS_NAMESPACE(506),
- AFTER_CHECK_EXISTS_NAMESPACE(507),
- BEFORE_DROP_NAMESPACE(508),
- AFTER_DROP_NAMESPACE(509),
- BEFORE_UPDATE_NAMESPACE_PROPERTIES(510),
- AFTER_UPDATE_NAMESPACE_PROPERTIES(511),
+ BEFORE_CREATE_NAMESPACE(500, Category.NAMESPACE),
+ AFTER_CREATE_NAMESPACE(501, Category.NAMESPACE),
+ BEFORE_LIST_NAMESPACES(502, Category.NAMESPACE),
+ AFTER_LIST_NAMESPACES(503, Category.NAMESPACE),
+ BEFORE_LOAD_NAMESPACE_METADATA(504, Category.NAMESPACE),
+ AFTER_LOAD_NAMESPACE_METADATA(505, Category.NAMESPACE),
+ BEFORE_CHECK_EXISTS_NAMESPACE(506, Category.NAMESPACE),
+ AFTER_CHECK_EXISTS_NAMESPACE(507, Category.NAMESPACE),
+ BEFORE_DROP_NAMESPACE(508, Category.NAMESPACE),
+ AFTER_DROP_NAMESPACE(509, Category.NAMESPACE),
+ BEFORE_UPDATE_NAMESPACE_PROPERTIES(510, Category.NAMESPACE),
+ AFTER_UPDATE_NAMESPACE_PROPERTIES(511, Category.NAMESPACE),
// Table Events
- BEFORE_CREATE_TABLE(600),
- AFTER_CREATE_TABLE(601),
- BEFORE_LIST_TABLES(602),
- AFTER_LIST_TABLES(603),
- BEFORE_LOAD_TABLE(604),
- AFTER_LOAD_TABLE(605),
- BEFORE_CHECK_EXISTS_TABLE(606),
- AFTER_CHECK_EXISTS_TABLE(607),
- BEFORE_DROP_TABLE(608),
- AFTER_DROP_TABLE(609),
- BEFORE_REGISTER_TABLE(610),
- AFTER_REGISTER_TABLE(611),
- BEFORE_RENAME_TABLE(612),
- AFTER_RENAME_TABLE(613),
- BEFORE_UPDATE_TABLE(614),
- AFTER_UPDATE_TABLE(615),
- BEFORE_REFRESH_TABLE(616),
- AFTER_REFRESH_TABLE(617),
+ BEFORE_CREATE_TABLE(600, Category.TABLE),
+ AFTER_CREATE_TABLE(601, Category.TABLE),
+ BEFORE_LIST_TABLES(602, Category.TABLE),
+ AFTER_LIST_TABLES(603, Category.TABLE),
+ BEFORE_LOAD_TABLE(604, Category.TABLE),
+ AFTER_LOAD_TABLE(605, Category.TABLE),
+ BEFORE_CHECK_EXISTS_TABLE(606, Category.TABLE),
+ AFTER_CHECK_EXISTS_TABLE(607, Category.TABLE),
+ BEFORE_DROP_TABLE(608, Category.TABLE),
+ AFTER_DROP_TABLE(609, Category.TABLE),
+ BEFORE_REGISTER_TABLE(610, Category.TABLE),
+ AFTER_REGISTER_TABLE(611, Category.TABLE),
+ BEFORE_RENAME_TABLE(612, Category.TABLE),
+ AFTER_RENAME_TABLE(613, Category.TABLE),
+ BEFORE_UPDATE_TABLE(614, Category.TABLE),
+ AFTER_UPDATE_TABLE(615, Category.TABLE),
+ BEFORE_REFRESH_TABLE(616, Category.TABLE),
+ AFTER_REFRESH_TABLE(617, Category.TABLE),
// View Events
- BEFORE_CREATE_VIEW(700),
- AFTER_CREATE_VIEW(701),
- BEFORE_LIST_VIEWS(702),
- AFTER_LIST_VIEWS(703),
- BEFORE_LOAD_VIEW(704),
- AFTER_LOAD_VIEW(705),
- BEFORE_CHECK_EXISTS_VIEW(706),
- AFTER_CHECK_EXISTS_VIEW(707),
- BEFORE_DROP_VIEW(708),
- AFTER_DROP_VIEW(709),
- BEFORE_RENAME_VIEW(710),
- AFTER_RENAME_VIEW(711),
- BEFORE_REPLACE_VIEW(712),
- AFTER_REPLACE_VIEW(713),
- BEFORE_REFRESH_VIEW(714),
- AFTER_REFRESH_VIEW(715),
+ BEFORE_CREATE_VIEW(700, Category.VIEW),
+ AFTER_CREATE_VIEW(701, Category.VIEW),
+ BEFORE_LIST_VIEWS(702, Category.VIEW),
+ AFTER_LIST_VIEWS(703, Category.VIEW),
+ BEFORE_LOAD_VIEW(704, Category.VIEW),
+ AFTER_LOAD_VIEW(705, Category.VIEW),
+ BEFORE_CHECK_EXISTS_VIEW(706, Category.VIEW),
+ AFTER_CHECK_EXISTS_VIEW(707, Category.VIEW),
+ BEFORE_DROP_VIEW(708, Category.VIEW),
+ AFTER_DROP_VIEW(709, Category.VIEW),
+ BEFORE_RENAME_VIEW(710, Category.VIEW),
+ AFTER_RENAME_VIEW(711, Category.VIEW),
+ BEFORE_REPLACE_VIEW(712, Category.VIEW),
+ AFTER_REPLACE_VIEW(713, Category.VIEW),
+ BEFORE_REFRESH_VIEW(714, Category.VIEW),
+ AFTER_REFRESH_VIEW(715, Category.VIEW),
// Credential Events
- BEFORE_LOAD_CREDENTIALS(800),
- AFTER_LOAD_CREDENTIALS(801),
+ BEFORE_LOAD_CREDENTIALS(800, Category.CREDENTIAL),
+ AFTER_LOAD_CREDENTIALS(801, Category.CREDENTIAL),
// Transaction Events
- BEFORE_COMMIT_TRANSACTION(900),
- AFTER_COMMIT_TRANSACTION(901),
+ BEFORE_COMMIT_TRANSACTION(900, Category.TRANSACTION),
+ AFTER_COMMIT_TRANSACTION(901, Category.TRANSACTION),
// Notification Events
- BEFORE_SEND_NOTIFICATION(1000),
- AFTER_SEND_NOTIFICATION(1001),
+ BEFORE_SEND_NOTIFICATION(1000, Category.NOTIFICATION),
+ AFTER_SEND_NOTIFICATION(1001, Category.NOTIFICATION),
// Configuration Events
- BEFORE_GET_CONFIG(1100),
- AFTER_GET_CONFIG(1101),
+ BEFORE_GET_CONFIG(1100, Category.CONFIG),
+ AFTER_GET_CONFIG(1101, Category.CONFIG),
// Policy Events
- BEFORE_CREATE_POLICY(1200),
- AFTER_CREATE_POLICY(1201),
- BEFORE_LIST_POLICIES(1202),
- AFTER_LIST_POLICIES(1203),
- BEFORE_LOAD_POLICY(1204),
- AFTER_LOAD_POLICY(1205),
- BEFORE_UPDATE_POLICY(1206),
- AFTER_UPDATE_POLICY(1207),
- BEFORE_DROP_POLICY(1208),
- AFTER_DROP_POLICY(1209),
- BEFORE_ATTACH_POLICY(1210),
- AFTER_ATTACH_POLICY(1211),
- BEFORE_DETACH_POLICY(1212),
- AFTER_DETACH_POLICY(1213),
- BEFORE_GET_APPLICABLE_POLICIES(1214),
- AFTER_GET_APPLICABLE_POLICIES(1215),
+ BEFORE_CREATE_POLICY(1200, Category.POLICY),
+ AFTER_CREATE_POLICY(1201, Category.POLICY),
+ BEFORE_LIST_POLICIES(1202, Category.POLICY),
+ AFTER_LIST_POLICIES(1203, Category.POLICY),
+ BEFORE_LOAD_POLICY(1204, Category.POLICY),
+ AFTER_LOAD_POLICY(1205, Category.POLICY),
+ BEFORE_UPDATE_POLICY(1206, Category.POLICY),
+ AFTER_UPDATE_POLICY(1207, Category.POLICY),
+ BEFORE_DROP_POLICY(1208, Category.POLICY),
+ AFTER_DROP_POLICY(1209, Category.POLICY),
+ BEFORE_ATTACH_POLICY(1210, Category.POLICY),
+ AFTER_ATTACH_POLICY(1211, Category.POLICY),
+ BEFORE_DETACH_POLICY(1212, Category.POLICY),
+ AFTER_DETACH_POLICY(1213, Category.POLICY),
+ BEFORE_GET_APPLICABLE_POLICIES(1214, Category.POLICY),
+ AFTER_GET_APPLICABLE_POLICIES(1215, Category.POLICY),
// Generic Table Events
- BEFORE_CREATE_GENERIC_TABLE(1300),
- AFTER_CREATE_GENERIC_TABLE(1301),
- BEFORE_DROP_GENERIC_TABLE(1302),
- AFTER_DROP_GENERIC_TABLE(1303),
- BEFORE_LIST_GENERIC_TABLES(1304),
- AFTER_LIST_GENERIC_TABLES(1305),
- BEFORE_LOAD_GENERIC_TABLE(1306),
- AFTER_LOAD_GENERIC_TABLE(1307),
+ BEFORE_CREATE_GENERIC_TABLE(1300, Category.GENERIC_TABLE),
+ AFTER_CREATE_GENERIC_TABLE(1301, Category.GENERIC_TABLE),
+ BEFORE_DROP_GENERIC_TABLE(1302, Category.GENERIC_TABLE),
+ AFTER_DROP_GENERIC_TABLE(1303, Category.GENERIC_TABLE),
+ BEFORE_LIST_GENERIC_TABLES(1304, Category.GENERIC_TABLE),
+ AFTER_LIST_GENERIC_TABLES(1305, Category.GENERIC_TABLE),
+ BEFORE_LOAD_GENERIC_TABLE(1306, Category.GENERIC_TABLE),
+ AFTER_LOAD_GENERIC_TABLE(1307, Category.GENERIC_TABLE),
// Task Execution Events
- BEFORE_ATTEMPT_TASK(1400),
- AFTER_ATTEMPT_TASK(1401),
+ BEFORE_ATTEMPT_TASK(1400, Category.TASK_EXECUTION),
+ AFTER_ATTEMPT_TASK(1401, Category.TASK_EXECUTION),
// Rate Limiting Events
- BEFORE_LIMIT_REQUEST_RATE(1500);
+ BEFORE_LIMIT_REQUEST_RATE(1500, Category.RATE_LIMITING);
private final int code;
+ private final Category category;
- private PolarisEventType(int code) {
+ PolarisEventType(int code, Category category) {
this.code = code;
+ this.category = category;
}
public int code() {
return code;
}
+
+ public Category category() {
+ return category;
+ }
+
+ public static Set<PolarisEventType> typesOfCategory(Category category) {
Review Comment:
nit: we could probably pre-compute this on startup to reduce this filter+collect logic during runtime since this will always be static.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3071619992
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
- Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes =
+ listenerConfiguration == null
Review Comment:
Despite this doesn't seem to be a problem now, it is reasonable to expect that when someone extends `ListenerConfiguration`, then there should be no need to list every event type. How about this:
```
var supportedTypes =
listenerConfiguration == null || (listenerConfiguration.enabledEventCategories().isEmpty() && listenerConfiguration.enabledEventTypes().isEmpty())
? EnumSet.allOf(PolarisEventType.class)
: EnumSet.noneOf(PolarisEventType.class);
```
This should make it future-proof though I don't think we can cover this with a test case, as `ListenerConfiguration` has only these two configs now.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4238963173
@nandorKollar : from CI: `ERROR: Configuration reference is out of date. Please run './gradlew :polaris-config-docs-site:copyConfigSectionsToSite' and commit the changes.`
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3032639218
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Ideally, we should use AOP for events, with Quarkus `@AroundInvoke` and introducing an `@PolarisEvent` annotation we could make this simple and clean. The problem is that I couldn't find a way to pass the event attribute map.
I'm also not too happy with the bolierplate if-s, it is very easy to forget that later when a new event is added, however I don't know what could be a better alternative. Like @adutra mentioned, using lambdas is also suboptimal approach.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3058718884
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,259 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.awaitility.Awaitility.await;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ PolarisEventType.typesOfCategory(PolarisEventType.Category.CATALOG);
+
+ private abstract static class FilteringEventListener implements PolarisEventListener {
+ private final Predicate<PolarisEvent> predicate;
+
+ List<PolarisEvent> expectedEvents = new CopyOnWriteArrayList<>();
+ List<PolarisEvent> unexpectedEvents = new CopyOnWriteArrayList<>();
+ AtomicInteger receivedEventCount = new AtomicInteger();
+
+ FilteringEventListener(Predicate<PolarisEvent> predicate) {
+ this.predicate = predicate;
+ }
+
+ @Override
+ public void onEvent(PolarisEvent event) {
+ if (predicate.test(event)) {
+ expectedEvents.add(event);
+ } else {
+ unexpectedEvents.add(event);
+ }
+ receivedEventCount.incrementAndGet();
+ }
+ }
+
+ private abstract static class BaseConsumeAllEventListener implements PolarisEventListener {
+ List<PolarisEvent> consumedEvents = new CopyOnWriteArrayList<>();
+ AtomicInteger receivedEventCount = new AtomicInteger();
+
+ @Override
+ public void onEvent(PolarisEvent event) {
+ consumedEvents.add(event);
+ receivedEventCount.incrementAndGet();
+ }
+ }
+
+ @Singleton
+ @Identifier("after-send-listener")
+ public static class AfterSendEventListener extends FilteringEventListener {
+ AfterSendEventListener() {
+ super(event -> event.type() == PolarisEventType.AFTER_SEND_NOTIFICATION);
+ }
+ }
+
+ @Singleton
+ @Identifier("before-send-listener")
+ public static class BeforeSendEventListener extends FilteringEventListener {
+ BeforeSendEventListener() {
+ super(event -> event.type() == PolarisEventType.BEFORE_SEND_NOTIFICATION);
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-all-listener")
+ public static class ConsumeAllEventListener extends BaseConsumeAllEventListener {}
+
+ @Singleton
+ @Identifier("consume-all-listener-2")
+ public static class ConsumeAllEventListener2 extends BaseConsumeAllEventListener {}
+
+ @Singleton
+ @Identifier("consume-only-catalog-listener")
+ public static class ConsumeOnlyCatalogEventsListener extends FilteringEventListener {
+ ConsumeOnlyCatalogEventsListener() {
+ super(event -> event.type().category() == PolarisEventType.Category.CATALOG);
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-catalog-and-after-notification-listener")
+ public static class ConsumeCatalogAndNotificationEventsListener extends FilteringEventListener {
+ ConsumeCatalogAndNotificationEventsListener() {
+ super(
+ event ->
+ event.type().category() == PolarisEventType.Category.CATALOG
+ || event.type() == PolarisEventType.AFTER_SEND_NOTIFICATION);
+ }
+ }
+
+ public static class PolarisEventListenersTestProfile implements QuarkusTestProfile {
+ @Override
+ public Map<String, String> getConfigOverrides() {
+ return ImmutableMap.<String, String>builder()
+ .put(
+ "polaris.event-listener.types",
+ "after-send-listener,before-send-listener,consume-all-listener,consume-all-listener-2,consume-only-catalog-listener,consume-catalog-and-after-notification-listener")
+ .put(
+ "polaris.event-listener.after-send-listener.enabled-event-types",
+ "AFTER_SEND_NOTIFICATION")
+ .put(
+ "polaris.event-listener.before-send-listener.enabled-event-types",
+ "BEFORE_SEND_NOTIFICATION")
+ .put(
+ "polaris.event-listener.consume-only-catalog-listener.enabled-event-categories",
+ "CATALOG")
+ .put(
+ "polaris.event-listener.consume-catalog-and-after-notification-listener.enabled-event-categories",
+ "CATALOG")
+ .put(
+ "polaris.event-listener.consume-catalog-and-after-notification-listener.enabled-event-types",
+ "AFTER_SEND_NOTIFICATION")
+ .build();
+ }
+ }
+
+ @Inject PolarisEventDispatcher eventDispatcher;
+
+ @Inject
+ @Identifier("after-send-listener")
+ PolarisEventListener afterSendEventListener;
+
+ @Inject
+ @Identifier("before-send-listener")
+ PolarisEventListener beforeSendEventListener;
+
+ @Inject
+ @Identifier("consume-all-listener")
+ PolarisEventListener consumeAllEventListener;
+
+ @Inject
+ @Identifier("consume-all-listener-2")
+ PolarisEventListener consumeAllEventListener2;
+
+ @Inject
+ @Identifier("consume-only-catalog-listener")
+ PolarisEventListener consumeOnlyCatalogEventListener;
+
+ @Inject
+ @Identifier("consume-catalog-and-after-notification-listener")
+ PolarisEventListener consumeCatalogAndNotificationEventListener;
+
+ @Test
+ public void testEventListenersGetNotified() {
+ var afterSendEventListener = (AfterSendEventListener) this.afterSendEventListener;
+ var beforeSendEventListener = (BeforeSendEventListener) this.beforeSendEventListener;
+ var consumeAllEventListener = (ConsumeAllEventListener) this.consumeAllEventListener;
+ var consumeAllEventListener2 = (ConsumeAllEventListener2) this.consumeAllEventListener2;
+ var consumeOnlyCatalogEventListener =
+ (ConsumeOnlyCatalogEventsListener) this.consumeOnlyCatalogEventListener;
+ var consumeCatalogAndNotificationEventListener =
+ (ConsumeCatalogAndNotificationEventsListener)
+ this.consumeCatalogAndNotificationEventListener;
+
+ eventDispatcher.dispatch(new PolarisEvent(PolarisEventType.AFTER_SEND_NOTIFICATION, null));
+ eventDispatcher.dispatch(new PolarisEvent(PolarisEventType.BEFORE_SEND_NOTIFICATION, null));
+ for (var eventType : CATALOG_EVENTS) {
+ eventDispatcher.dispatch(new PolarisEvent(eventType, null));
+ }
+
+ // Only after send notification event
+ await().until(() -> afterSendEventListener.receivedEventCount.get() == 1);
+ assertEquals(1, afterSendEventListener.expectedEvents.size());
+ assertEquals(
+ PolarisEventType.AFTER_SEND_NOTIFICATION,
+ afterSendEventListener.expectedEvents.getFirst().type());
+
+ // Only before send notification event
+ await().until(() -> beforeSendEventListener.receivedEventCount.get() == 1);
+ assertEquals(1, beforeSendEventListener.expectedEvents.size());
+ assertEquals(
+ PolarisEventType.BEFORE_SEND_NOTIFICATION,
+ beforeSendEventListener.expectedEvents.getFirst().type());
+
+ // All events received by both consumeAllEventListener and consumeAllEventListener2
+ for (var listener :
+ new BaseConsumeAllEventListener[] {consumeAllEventListener, consumeAllEventListener2}) {
+ await().until(() -> listener.receivedEventCount.get() == 2 + CATALOG_EVENTS.size());
+ assertEquals(2 + CATALOG_EVENTS.size(), listener.consumedEvents.size());
+ expectEvents(
+ listener.consumedEvents,
+ Stream.concat(
+ Stream.of(
+ PolarisEventType.AFTER_SEND_NOTIFICATION,
+ PolarisEventType.BEFORE_SEND_NOTIFICATION),
+ CATALOG_EVENTS.stream())
+ .collect(Collectors.toSet()));
+ }
Review Comment:
Minor suggestion to improve readability:
```suggestion
List<PolarisEvent> allEvents = new ArrayList<>();
allEvents.add(new PolarisEvent(PolarisEventType.AFTER_SEND_NOTIFICATION, null));
allEvents.add(new PolarisEvent(PolarisEventType.BEFORE_SEND_NOTIFICATION, null));
CATALOG_EVENTS.forEach(eventType -> allEvents.add(new PolarisEvent(eventType, null)));
allEvents.forEach(event -> eventDispatcher.dispatch(event));
// Only after send notification event
await().until(() -> afterSendEventListener.receivedEventCount.get() == 1);
assertEquals(1, afterSendEventListener.expectedEvents.size());
assertEquals(
PolarisEventType.AFTER_SEND_NOTIFICATION,
afterSendEventListener.expectedEvents.getFirst().type());
// Only before send notification event
await().until(() -> beforeSendEventListener.receivedEventCount.get() == 1);
assertEquals(1, beforeSendEventListener.expectedEvents.size());
assertEquals(
PolarisEventType.BEFORE_SEND_NOTIFICATION,
beforeSendEventListener.expectedEvents.getFirst().type());
// All events received by both consumeAllEventListener and consumeAllEventListener2
for (var listener :
new BaseConsumeAllEventListener[] {consumeAllEventListener, consumeAllEventListener2}) {
await().until(() -> listener.receivedEventCount.get() == allEvents.size());
assertEquals(allEvents.size(), listener.consumedEvents.size());
expectEvents(
listener.consumedEvents,
allEvents.stream().map(PolarisEvent::type).collect(Collectors.toSet()));
}
```
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,259 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.awaitility.Awaitility.await;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ PolarisEventType.typesOfCategory(PolarisEventType.Category.CATALOG);
+
+ private abstract static class FilteringEventListener implements PolarisEventListener {
+ private final Predicate<PolarisEvent> predicate;
+
+ List<PolarisEvent> expectedEvents = new CopyOnWriteArrayList<>();
+ List<PolarisEvent> unexpectedEvents = new CopyOnWriteArrayList<>();
+ AtomicInteger receivedEventCount = new AtomicInteger();
+
+ FilteringEventListener(Predicate<PolarisEvent> predicate) {
+ this.predicate = predicate;
+ }
+
+ @Override
+ public void onEvent(PolarisEvent event) {
+ if (predicate.test(event)) {
+ expectedEvents.add(event);
+ } else {
+ unexpectedEvents.add(event);
+ }
+ receivedEventCount.incrementAndGet();
+ }
+ }
+
+ private abstract static class BaseConsumeAllEventListener implements PolarisEventListener {
+ List<PolarisEvent> consumedEvents = new CopyOnWriteArrayList<>();
+ AtomicInteger receivedEventCount = new AtomicInteger();
+
+ @Override
+ public void onEvent(PolarisEvent event) {
+ consumedEvents.add(event);
+ receivedEventCount.incrementAndGet();
+ }
+ }
+
+ @Singleton
+ @Identifier("after-send-listener")
+ public static class AfterSendEventListener extends FilteringEventListener {
+ AfterSendEventListener() {
+ super(event -> event.type() == PolarisEventType.AFTER_SEND_NOTIFICATION);
+ }
+ }
+
+ @Singleton
+ @Identifier("before-send-listener")
+ public static class BeforeSendEventListener extends FilteringEventListener {
+ BeforeSendEventListener() {
+ super(event -> event.type() == PolarisEventType.BEFORE_SEND_NOTIFICATION);
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-all-listener")
+ public static class ConsumeAllEventListener extends BaseConsumeAllEventListener {}
+
+ @Singleton
+ @Identifier("consume-all-listener-2")
+ public static class ConsumeAllEventListener2 extends BaseConsumeAllEventListener {}
+
+ @Singleton
+ @Identifier("consume-only-catalog-listener")
+ public static class ConsumeOnlyCatalogEventsListener extends FilteringEventListener {
+ ConsumeOnlyCatalogEventsListener() {
+ super(event -> event.type().category() == PolarisEventType.Category.CATALOG);
+ }
+ }
+
+ @Singleton
+ @Identifier("consume-catalog-and-after-notification-listener")
+ public static class ConsumeCatalogAndNotificationEventsListener extends FilteringEventListener {
+ ConsumeCatalogAndNotificationEventsListener() {
+ super(
+ event ->
+ event.type().category() == PolarisEventType.Category.CATALOG
+ || event.type() == PolarisEventType.AFTER_SEND_NOTIFICATION);
+ }
+ }
+
+ public static class PolarisEventListenersTestProfile implements QuarkusTestProfile {
+ @Override
+ public Map<String, String> getConfigOverrides() {
+ return ImmutableMap.<String, String>builder()
+ .put(
+ "polaris.event-listener.types",
+ "after-send-listener,before-send-listener,consume-all-listener,consume-all-listener-2,consume-only-catalog-listener,consume-catalog-and-after-notification-listener")
+ .put(
+ "polaris.event-listener.after-send-listener.enabled-event-types",
+ "AFTER_SEND_NOTIFICATION")
+ .put(
+ "polaris.event-listener.before-send-listener.enabled-event-types",
+ "BEFORE_SEND_NOTIFICATION")
+ .put(
+ "polaris.event-listener.consume-only-catalog-listener.enabled-event-categories",
+ "CATALOG")
+ .put(
+ "polaris.event-listener.consume-catalog-and-after-notification-listener.enabled-event-categories",
+ "CATALOG")
+ .put(
+ "polaris.event-listener.consume-catalog-and-after-notification-listener.enabled-event-types",
+ "AFTER_SEND_NOTIFICATION")
+ .build();
+ }
+ }
+
+ @Inject PolarisEventDispatcher eventDispatcher;
+
+ @Inject
+ @Identifier("after-send-listener")
+ PolarisEventListener afterSendEventListener;
+
+ @Inject
+ @Identifier("before-send-listener")
+ PolarisEventListener beforeSendEventListener;
+
+ @Inject
+ @Identifier("consume-all-listener")
+ PolarisEventListener consumeAllEventListener;
+
+ @Inject
+ @Identifier("consume-all-listener-2")
+ PolarisEventListener consumeAllEventListener2;
+
+ @Inject
+ @Identifier("consume-only-catalog-listener")
+ PolarisEventListener consumeOnlyCatalogEventListener;
+
+ @Inject
+ @Identifier("consume-catalog-and-after-notification-listener")
+ PolarisEventListener consumeCatalogAndNotificationEventListener;
+
+ @Test
+ public void testEventListenersGetNotified() {
+ var afterSendEventListener = (AfterSendEventListener) this.afterSendEventListener;
+ var beforeSendEventListener = (BeforeSendEventListener) this.beforeSendEventListener;
+ var consumeAllEventListener = (ConsumeAllEventListener) this.consumeAllEventListener;
+ var consumeAllEventListener2 = (ConsumeAllEventListener2) this.consumeAllEventListener2;
+ var consumeOnlyCatalogEventListener =
+ (ConsumeOnlyCatalogEventsListener) this.consumeOnlyCatalogEventListener;
+ var consumeCatalogAndNotificationEventListener =
+ (ConsumeCatalogAndNotificationEventsListener)
+ this.consumeCatalogAndNotificationEventListener;
+
+ eventDispatcher.dispatch(new PolarisEvent(PolarisEventType.AFTER_SEND_NOTIFICATION, null));
+ eventDispatcher.dispatch(new PolarisEvent(PolarisEventType.BEFORE_SEND_NOTIFICATION, null));
+ for (var eventType : CATALOG_EVENTS) {
+ eventDispatcher.dispatch(new PolarisEvent(eventType, null));
+ }
+
+ // Only after send notification event
+ await().until(() -> afterSendEventListener.receivedEventCount.get() == 1);
+ assertEquals(1, afterSendEventListener.expectedEvents.size());
+ assertEquals(
+ PolarisEventType.AFTER_SEND_NOTIFICATION,
+ afterSendEventListener.expectedEvents.getFirst().type());
+
+ // Only before send notification event
+ await().until(() -> beforeSendEventListener.receivedEventCount.get() == 1);
+ assertEquals(1, beforeSendEventListener.expectedEvents.size());
+ assertEquals(
+ PolarisEventType.BEFORE_SEND_NOTIFICATION,
+ beforeSendEventListener.expectedEvents.getFirst().type());
+
+ // All events received by both consumeAllEventListener and consumeAllEventListener2
+ for (var listener :
+ new BaseConsumeAllEventListener[] {consumeAllEventListener, consumeAllEventListener2}) {
+ await().until(() -> listener.receivedEventCount.get() == 2 + CATALOG_EVENTS.size());
+ assertEquals(2 + CATALOG_EVENTS.size(), listener.consumedEvents.size());
+ expectEvents(
+ listener.consumedEvents,
+ Stream.concat(
+ Stream.of(
+ PolarisEventType.AFTER_SEND_NOTIFICATION,
+ PolarisEventType.BEFORE_SEND_NOTIFICATION),
+ CATALOG_EVENTS.stream())
+ .collect(Collectors.toSet()));
+ }
+
+ // Only catalog events
+ await()
+ .until(
+ () ->
+ consumeOnlyCatalogEventListener.receivedEventCount.get() == CATALOG_EVENTS.size());
+ assertEquals(CATALOG_EVENTS.size(), consumeOnlyCatalogEventListener.expectedEvents.size());
+ expectEvents(consumeOnlyCatalogEventListener.expectedEvents, CATALOG_EVENTS);
+
+ // Catalog and after send notification events
+ await()
+ .until(
+ () ->
+ consumeCatalogAndNotificationEventListener.receivedEventCount.get()
+ == CATALOG_EVENTS.size() + 1);
+ assertEquals(
+ CATALOG_EVENTS.size() + 1,
+ consumeCatalogAndNotificationEventListener.expectedEvents.size());
+ expectEvents(
+ consumeCatalogAndNotificationEventListener.expectedEvents,
+ Stream.concat(Stream.of(PolarisEventType.AFTER_SEND_NOTIFICATION), CATALOG_EVENTS.stream())
+ .collect(Collectors.toSet()));
+
+ // There are no unexpected events
+ assertTrue(afterSendEventListener.unexpectedEvents.isEmpty());
+ assertTrue(beforeSendEventListener.unexpectedEvents.isEmpty());
+ assertTrue(consumeOnlyCatalogEventListener.unexpectedEvents.isEmpty());
+ assertTrue(consumeCatalogAndNotificationEventListener.unexpectedEvents.isEmpty());
+ }
+
+ private void expectEvents(List<PolarisEvent> events, Set<PolarisEventType> expectedTypes) {
+ for (var event : expectedTypes) {
Review Comment:
nit:
```suggestion
for (var eventType : expectedTypes) {
```
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani merged PR #4064:
URL: https://github.com/apache/polaris/pull/4064
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3051802463
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ Arrays.stream(PolarisEventType.values())
+ .filter(e -> e.category() == PolarisEventType.Category.CATALOG)
+ .collect(Collectors.toSet());
+
+ static final CountDownLatch latch = new CountDownLatch(5 + 3 * CATALOG_EVENTS.size());
Review Comment:
I haven't heard about Awaitility, is it this one: https://github.com/awaitility/awaitility? It would require an extra dependency, do we need that? I agree the latch size looks indeed magic, we can refactor that to a constant, and write a proper comment for it.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3071745145
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java:
##########
@@ -19,189 +19,223 @@
package org.apache.polaris.service.events;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
public enum PolarisEventType {
// Catalog Events
- BEFORE_CREATE_CATALOG(100),
- AFTER_CREATE_CATALOG(101),
- BEFORE_DELETE_CATALOG(102),
- AFTER_DELETE_CATALOG(103),
- BEFORE_GET_CATALOG(104),
- AFTER_GET_CATALOG(105),
- BEFORE_UPDATE_CATALOG(106),
- AFTER_UPDATE_CATALOG(107),
- BEFORE_LIST_CATALOGS(108),
- AFTER_LIST_CATALOGS(109),
+ BEFORE_CREATE_CATALOG(100, Category.CATALOG),
+ AFTER_CREATE_CATALOG(101, Category.CATALOG),
+ BEFORE_DELETE_CATALOG(102, Category.CATALOG),
+ AFTER_DELETE_CATALOG(103, Category.CATALOG),
+ BEFORE_GET_CATALOG(104, Category.CATALOG),
+ AFTER_GET_CATALOG(105, Category.CATALOG),
+ BEFORE_UPDATE_CATALOG(106, Category.CATALOG),
+ AFTER_UPDATE_CATALOG(107, Category.CATALOG),
+ BEFORE_LIST_CATALOGS(108, Category.CATALOG),
+ AFTER_LIST_CATALOGS(109, Category.CATALOG),
// Catalog Role Events
- BEFORE_CREATE_CATALOG_ROLE(200),
- AFTER_CREATE_CATALOG_ROLE(201),
- BEFORE_DELETE_CATALOG_ROLE(202),
- AFTER_DELETE_CATALOG_ROLE(203),
- BEFORE_GET_CATALOG_ROLE(204),
- AFTER_GET_CATALOG_ROLE(205),
- BEFORE_UPDATE_CATALOG_ROLE(206),
- AFTER_UPDATE_CATALOG_ROLE(207),
- BEFORE_LIST_CATALOG_ROLES(208),
- AFTER_LIST_CATALOG_ROLES(209),
- BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210),
- AFTER_ADD_GRANT_TO_CATALOG_ROLE(211),
- BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212),
- AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213),
- BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214),
- AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215),
- BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216),
- AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217),
+ BEFORE_CREATE_CATALOG_ROLE(200, Category.CATALOG_ROLE),
+ AFTER_CREATE_CATALOG_ROLE(201, Category.CATALOG_ROLE),
+ BEFORE_DELETE_CATALOG_ROLE(202, Category.CATALOG_ROLE),
+ AFTER_DELETE_CATALOG_ROLE(203, Category.CATALOG_ROLE),
+ BEFORE_GET_CATALOG_ROLE(204, Category.CATALOG_ROLE),
+ AFTER_GET_CATALOG_ROLE(205, Category.CATALOG_ROLE),
+ BEFORE_UPDATE_CATALOG_ROLE(206, Category.CATALOG_ROLE),
+ AFTER_UPDATE_CATALOG_ROLE(207, Category.CATALOG_ROLE),
+ BEFORE_LIST_CATALOG_ROLES(208, Category.CATALOG_ROLE),
+ AFTER_LIST_CATALOG_ROLES(209, Category.CATALOG_ROLE),
+ BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210, Category.CATALOG_ROLE),
+ AFTER_ADD_GRANT_TO_CATALOG_ROLE(211, Category.CATALOG_ROLE),
+ BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212, Category.CATALOG_ROLE),
+ AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213, Category.CATALOG_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214, Category.CATALOG_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215, Category.CATALOG_ROLE),
+ BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216, Category.CATALOG_ROLE),
+ AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217, Category.CATALOG_ROLE),
// Principal Events
- BEFORE_CREATE_PRINCIPAL(300),
- AFTER_CREATE_PRINCIPAL(301),
- BEFORE_DELETE_PRINCIPAL(302),
- AFTER_DELETE_PRINCIPAL(303),
- BEFORE_GET_PRINCIPAL(304),
- AFTER_GET_PRINCIPAL(305),
- BEFORE_UPDATE_PRINCIPAL(306),
- AFTER_UPDATE_PRINCIPAL(307),
- BEFORE_ROTATE_CREDENTIALS(308),
- AFTER_ROTATE_CREDENTIALS(309),
- BEFORE_RESET_CREDENTIALS(310),
- AFTER_RESET_CREDENTIALS(311),
- BEFORE_LIST_PRINCIPALS(312),
- AFTER_LIST_PRINCIPALS(313),
- BEFORE_ASSIGN_PRINCIPAL_ROLE(314),
- AFTER_ASSIGN_PRINCIPAL_ROLE(315),
- BEFORE_REVOKE_PRINCIPAL_ROLE(316),
- AFTER_REVOKE_PRINCIPAL_ROLE(317),
- BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318),
- AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319),
+ BEFORE_CREATE_PRINCIPAL(300, Category.PRINCIPAL),
+ AFTER_CREATE_PRINCIPAL(301, Category.PRINCIPAL),
+ BEFORE_DELETE_PRINCIPAL(302, Category.PRINCIPAL),
+ AFTER_DELETE_PRINCIPAL(303, Category.PRINCIPAL),
+ BEFORE_GET_PRINCIPAL(304, Category.PRINCIPAL),
+ AFTER_GET_PRINCIPAL(305, Category.PRINCIPAL),
+ BEFORE_UPDATE_PRINCIPAL(306, Category.PRINCIPAL),
+ AFTER_UPDATE_PRINCIPAL(307, Category.PRINCIPAL),
+ BEFORE_ROTATE_CREDENTIALS(308, Category.PRINCIPAL),
+ AFTER_ROTATE_CREDENTIALS(309, Category.PRINCIPAL),
+ BEFORE_RESET_CREDENTIALS(310, Category.PRINCIPAL),
+ AFTER_RESET_CREDENTIALS(311, Category.PRINCIPAL),
+ BEFORE_LIST_PRINCIPALS(312, Category.PRINCIPAL),
+ AFTER_LIST_PRINCIPALS(313, Category.PRINCIPAL),
+ BEFORE_ASSIGN_PRINCIPAL_ROLE(314, Category.PRINCIPAL),
+ AFTER_ASSIGN_PRINCIPAL_ROLE(315, Category.PRINCIPAL),
+ BEFORE_REVOKE_PRINCIPAL_ROLE(316, Category.PRINCIPAL),
+ AFTER_REVOKE_PRINCIPAL_ROLE(317, Category.PRINCIPAL),
+ BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318, Category.PRINCIPAL),
+ AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319, Category.PRINCIPAL),
// Principal Role Events
- BEFORE_CREATE_PRINCIPAL_ROLE(400),
- AFTER_CREATE_PRINCIPAL_ROLE(401),
- BEFORE_DELETE_PRINCIPAL_ROLE(402),
- AFTER_DELETE_PRINCIPAL_ROLE(403),
- BEFORE_GET_PRINCIPAL_ROLE(404),
- AFTER_GET_PRINCIPAL_ROLE(405),
- BEFORE_UPDATE_PRINCIPAL_ROLE(406),
- AFTER_UPDATE_PRINCIPAL_ROLE(407),
- BEFORE_LIST_PRINCIPAL_ROLES(408),
- AFTER_LIST_PRINCIPAL_ROLES(409),
- BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410),
- AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411),
- BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412),
- AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413),
- BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414),
- AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415),
- BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416),
- AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417),
+ BEFORE_CREATE_PRINCIPAL_ROLE(400, Category.PRINCIPAL_ROLE),
+ AFTER_CREATE_PRINCIPAL_ROLE(401, Category.PRINCIPAL_ROLE),
+ BEFORE_DELETE_PRINCIPAL_ROLE(402, Category.PRINCIPAL_ROLE),
+ AFTER_DELETE_PRINCIPAL_ROLE(403, Category.PRINCIPAL_ROLE),
+ BEFORE_GET_PRINCIPAL_ROLE(404, Category.PRINCIPAL_ROLE),
+ AFTER_GET_PRINCIPAL_ROLE(405, Category.PRINCIPAL_ROLE),
+ BEFORE_UPDATE_PRINCIPAL_ROLE(406, Category.PRINCIPAL_ROLE),
+ AFTER_UPDATE_PRINCIPAL_ROLE(407, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_PRINCIPAL_ROLES(408, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_PRINCIPAL_ROLES(409, Category.PRINCIPAL_ROLE),
+ BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410, Category.PRINCIPAL_ROLE),
+ AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411, Category.PRINCIPAL_ROLE),
+ BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412, Category.PRINCIPAL_ROLE),
+ AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417, Category.PRINCIPAL_ROLE),
// Namespace Events
- BEFORE_CREATE_NAMESPACE(500),
- AFTER_CREATE_NAMESPACE(501),
- BEFORE_LIST_NAMESPACES(502),
- AFTER_LIST_NAMESPACES(503),
- BEFORE_LOAD_NAMESPACE_METADATA(504),
- AFTER_LOAD_NAMESPACE_METADATA(505),
- BEFORE_CHECK_EXISTS_NAMESPACE(506),
- AFTER_CHECK_EXISTS_NAMESPACE(507),
- BEFORE_DROP_NAMESPACE(508),
- AFTER_DROP_NAMESPACE(509),
- BEFORE_UPDATE_NAMESPACE_PROPERTIES(510),
- AFTER_UPDATE_NAMESPACE_PROPERTIES(511),
+ BEFORE_CREATE_NAMESPACE(500, Category.NAMESPACE),
+ AFTER_CREATE_NAMESPACE(501, Category.NAMESPACE),
+ BEFORE_LIST_NAMESPACES(502, Category.NAMESPACE),
+ AFTER_LIST_NAMESPACES(503, Category.NAMESPACE),
+ BEFORE_LOAD_NAMESPACE_METADATA(504, Category.NAMESPACE),
+ AFTER_LOAD_NAMESPACE_METADATA(505, Category.NAMESPACE),
+ BEFORE_CHECK_EXISTS_NAMESPACE(506, Category.NAMESPACE),
+ AFTER_CHECK_EXISTS_NAMESPACE(507, Category.NAMESPACE),
+ BEFORE_DROP_NAMESPACE(508, Category.NAMESPACE),
+ AFTER_DROP_NAMESPACE(509, Category.NAMESPACE),
+ BEFORE_UPDATE_NAMESPACE_PROPERTIES(510, Category.NAMESPACE),
+ AFTER_UPDATE_NAMESPACE_PROPERTIES(511, Category.NAMESPACE),
// Table Events
- BEFORE_CREATE_TABLE(600),
- AFTER_CREATE_TABLE(601),
- BEFORE_LIST_TABLES(602),
- AFTER_LIST_TABLES(603),
- BEFORE_LOAD_TABLE(604),
- AFTER_LOAD_TABLE(605),
- BEFORE_CHECK_EXISTS_TABLE(606),
- AFTER_CHECK_EXISTS_TABLE(607),
- BEFORE_DROP_TABLE(608),
- AFTER_DROP_TABLE(609),
- BEFORE_REGISTER_TABLE(610),
- AFTER_REGISTER_TABLE(611),
- BEFORE_RENAME_TABLE(612),
- AFTER_RENAME_TABLE(613),
- BEFORE_UPDATE_TABLE(614),
- AFTER_UPDATE_TABLE(615),
- BEFORE_REFRESH_TABLE(616),
- AFTER_REFRESH_TABLE(617),
+ BEFORE_CREATE_TABLE(600, Category.TABLE),
+ AFTER_CREATE_TABLE(601, Category.TABLE),
+ BEFORE_LIST_TABLES(602, Category.TABLE),
+ AFTER_LIST_TABLES(603, Category.TABLE),
+ BEFORE_LOAD_TABLE(604, Category.TABLE),
+ AFTER_LOAD_TABLE(605, Category.TABLE),
+ BEFORE_CHECK_EXISTS_TABLE(606, Category.TABLE),
+ AFTER_CHECK_EXISTS_TABLE(607, Category.TABLE),
+ BEFORE_DROP_TABLE(608, Category.TABLE),
+ AFTER_DROP_TABLE(609, Category.TABLE),
+ BEFORE_REGISTER_TABLE(610, Category.TABLE),
+ AFTER_REGISTER_TABLE(611, Category.TABLE),
+ BEFORE_RENAME_TABLE(612, Category.TABLE),
+ AFTER_RENAME_TABLE(613, Category.TABLE),
+ BEFORE_UPDATE_TABLE(614, Category.TABLE),
+ AFTER_UPDATE_TABLE(615, Category.TABLE),
+ BEFORE_REFRESH_TABLE(616, Category.TABLE),
+ AFTER_REFRESH_TABLE(617, Category.TABLE),
// View Events
- BEFORE_CREATE_VIEW(700),
- AFTER_CREATE_VIEW(701),
- BEFORE_LIST_VIEWS(702),
- AFTER_LIST_VIEWS(703),
- BEFORE_LOAD_VIEW(704),
- AFTER_LOAD_VIEW(705),
- BEFORE_CHECK_EXISTS_VIEW(706),
- AFTER_CHECK_EXISTS_VIEW(707),
- BEFORE_DROP_VIEW(708),
- AFTER_DROP_VIEW(709),
- BEFORE_RENAME_VIEW(710),
- AFTER_RENAME_VIEW(711),
- BEFORE_REPLACE_VIEW(712),
- AFTER_REPLACE_VIEW(713),
- BEFORE_REFRESH_VIEW(714),
- AFTER_REFRESH_VIEW(715),
+ BEFORE_CREATE_VIEW(700, Category.VIEW),
+ AFTER_CREATE_VIEW(701, Category.VIEW),
+ BEFORE_LIST_VIEWS(702, Category.VIEW),
+ AFTER_LIST_VIEWS(703, Category.VIEW),
+ BEFORE_LOAD_VIEW(704, Category.VIEW),
+ AFTER_LOAD_VIEW(705, Category.VIEW),
+ BEFORE_CHECK_EXISTS_VIEW(706, Category.VIEW),
+ AFTER_CHECK_EXISTS_VIEW(707, Category.VIEW),
+ BEFORE_DROP_VIEW(708, Category.VIEW),
+ AFTER_DROP_VIEW(709, Category.VIEW),
+ BEFORE_RENAME_VIEW(710, Category.VIEW),
+ AFTER_RENAME_VIEW(711, Category.VIEW),
+ BEFORE_REPLACE_VIEW(712, Category.VIEW),
+ AFTER_REPLACE_VIEW(713, Category.VIEW),
+ BEFORE_REFRESH_VIEW(714, Category.VIEW),
+ AFTER_REFRESH_VIEW(715, Category.VIEW),
// Credential Events
- BEFORE_LOAD_CREDENTIALS(800),
- AFTER_LOAD_CREDENTIALS(801),
+ BEFORE_LOAD_CREDENTIALS(800, Category.CREDENTIAL),
+ AFTER_LOAD_CREDENTIALS(801, Category.CREDENTIAL),
// Transaction Events
- BEFORE_COMMIT_TRANSACTION(900),
- AFTER_COMMIT_TRANSACTION(901),
+ BEFORE_COMMIT_TRANSACTION(900, Category.TRANSACTION),
+ AFTER_COMMIT_TRANSACTION(901, Category.TRANSACTION),
// Notification Events
- BEFORE_SEND_NOTIFICATION(1000),
- AFTER_SEND_NOTIFICATION(1001),
+ BEFORE_SEND_NOTIFICATION(1000, Category.NOTIFICATION),
+ AFTER_SEND_NOTIFICATION(1001, Category.NOTIFICATION),
// Configuration Events
- BEFORE_GET_CONFIG(1100),
- AFTER_GET_CONFIG(1101),
+ BEFORE_GET_CONFIG(1100, Category.CONFIG),
+ AFTER_GET_CONFIG(1101, Category.CONFIG),
// Policy Events
- BEFORE_CREATE_POLICY(1200),
- AFTER_CREATE_POLICY(1201),
- BEFORE_LIST_POLICIES(1202),
- AFTER_LIST_POLICIES(1203),
- BEFORE_LOAD_POLICY(1204),
- AFTER_LOAD_POLICY(1205),
- BEFORE_UPDATE_POLICY(1206),
- AFTER_UPDATE_POLICY(1207),
- BEFORE_DROP_POLICY(1208),
- AFTER_DROP_POLICY(1209),
- BEFORE_ATTACH_POLICY(1210),
- AFTER_ATTACH_POLICY(1211),
- BEFORE_DETACH_POLICY(1212),
- AFTER_DETACH_POLICY(1213),
- BEFORE_GET_APPLICABLE_POLICIES(1214),
- AFTER_GET_APPLICABLE_POLICIES(1215),
+ BEFORE_CREATE_POLICY(1200, Category.POLICY),
+ AFTER_CREATE_POLICY(1201, Category.POLICY),
+ BEFORE_LIST_POLICIES(1202, Category.POLICY),
+ AFTER_LIST_POLICIES(1203, Category.POLICY),
+ BEFORE_LOAD_POLICY(1204, Category.POLICY),
+ AFTER_LOAD_POLICY(1205, Category.POLICY),
+ BEFORE_UPDATE_POLICY(1206, Category.POLICY),
+ AFTER_UPDATE_POLICY(1207, Category.POLICY),
+ BEFORE_DROP_POLICY(1208, Category.POLICY),
+ AFTER_DROP_POLICY(1209, Category.POLICY),
+ BEFORE_ATTACH_POLICY(1210, Category.POLICY),
+ AFTER_ATTACH_POLICY(1211, Category.POLICY),
+ BEFORE_DETACH_POLICY(1212, Category.POLICY),
+ AFTER_DETACH_POLICY(1213, Category.POLICY),
+ BEFORE_GET_APPLICABLE_POLICIES(1214, Category.POLICY),
+ AFTER_GET_APPLICABLE_POLICIES(1215, Category.POLICY),
// Generic Table Events
- BEFORE_CREATE_GENERIC_TABLE(1300),
- AFTER_CREATE_GENERIC_TABLE(1301),
- BEFORE_DROP_GENERIC_TABLE(1302),
- AFTER_DROP_GENERIC_TABLE(1303),
- BEFORE_LIST_GENERIC_TABLES(1304),
- AFTER_LIST_GENERIC_TABLES(1305),
- BEFORE_LOAD_GENERIC_TABLE(1306),
- AFTER_LOAD_GENERIC_TABLE(1307),
+ BEFORE_CREATE_GENERIC_TABLE(1300, Category.GENERIC_TABLE),
+ AFTER_CREATE_GENERIC_TABLE(1301, Category.GENERIC_TABLE),
+ BEFORE_DROP_GENERIC_TABLE(1302, Category.GENERIC_TABLE),
+ AFTER_DROP_GENERIC_TABLE(1303, Category.GENERIC_TABLE),
+ BEFORE_LIST_GENERIC_TABLES(1304, Category.GENERIC_TABLE),
+ AFTER_LIST_GENERIC_TABLES(1305, Category.GENERIC_TABLE),
+ BEFORE_LOAD_GENERIC_TABLE(1306, Category.GENERIC_TABLE),
+ AFTER_LOAD_GENERIC_TABLE(1307, Category.GENERIC_TABLE),
// Task Execution Events
- BEFORE_ATTEMPT_TASK(1400),
- AFTER_ATTEMPT_TASK(1401),
+ BEFORE_ATTEMPT_TASK(1400, Category.TASK_EXECUTION),
+ AFTER_ATTEMPT_TASK(1401, Category.TASK_EXECUTION),
// Rate Limiting Events
- BEFORE_LIMIT_REQUEST_RATE(1500);
+ BEFORE_LIMIT_REQUEST_RATE(1500, Category.RATE_LIMITING);
private final int code;
+ private final Category category;
- private PolarisEventType(int code) {
+ PolarisEventType(int code, Category category) {
this.code = code;
+ this.category = category;
}
public int code() {
return code;
}
+
+ public Category category() {
+ return category;
+ }
+
+ public static Set<PolarisEventType> typesOfCategory(Category category) {
Review Comment:
We can introduce a map to improve lookup speed, if that would help, something like this:
```
private static final Map<Category, EnumSet<PolarisEventType>> categoryToTypes = new HashMap<>();
static {
for (PolarisEventType type : values()) {
EnumSet<PolarisEventType> types = categoryToTypes.getOrDefault(type.category, EnumSet.noneOf(PolarisEventType.class));
types.add(type);
categoryToTypes.put(type.category, types);
}
}
```
and rewrite `typeOfCategory`:
```
public static Set<PolarisEventType> typesOfCategory(Category category) {
return categoryToTypes.get(category);
}
```
Did you think of something like this?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3032254467
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
> Maybe it would be cleaner to a helper like `dispatchIfListening(Supplier<PolarisEvent>)` or similar to simplify the maintenance.
You would still be creating a lambda + closure if you do this. Only the if block allows to completely avoid creating objects that would be discarded immediately if there are no listeners for them.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3075964035
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,69 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final EnumSet<PolarisEventType> eventsByType = EnumSet.allOf(PolarisEventType.class);
Review Comment:
nit: maybe `enabledTypes`?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3029187302
##########
site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_event_listener.md:
##########
@@ -25,5 +25,6 @@ build:
| Property | Default Value | Type | Description |
|----------|---------------|------|-------------|
-| `polaris.event-listener.type` | | `string` | The type of the event listener to use. Must be a registered (`PolarisEventListener`) identifier. <br><br>_Deprecated_ since 1.5.0, use 'polaris.event-listener.types' instead |
+| `polaris.event-listener.type` | | `string` | The type of the event listener to use. Must be a registered (`PolarisEventListener`) identifier. <br><br>_Deprecated_ since 1.5.0, use 'polaris.event-listener.types' instead, if both are set, then polaris.event-listener.types is prioritized |
| `polaris.event-listener.types` | | `list of string` | Comma separated list of event listers, each item must be a registered (`PolarisEventListener`) identifier. |
+| `polaris.event-listener.`_`<name>`_`.enabled-event-types` | | `list of BEFORE_CREATE_CATALOG, AFTER_CREATE_CATALOG, BEFORE_DELETE_CATALOG, AFTER_DELETE_CATALOG, BEFORE_GET_CATALOG, AFTER_GET_CATALOG, BEFORE_UPDATE_CATALOG, AFTER_UPDATE_CATALOG, BEFORE_LIST_CATALOGS, AFTER_LIST_CATALOGS, BEFORE_CREATE_CATALOG_ROLE, AFTER_CREATE_CATALOG_ROLE, BEFORE_DELETE_CATALOG_ROLE, AFTER_DELETE_CATALOG_ROLE, BEFORE_GET_CATALOG_ROLE, AFTER_GET_CATALOG_ROLE, BEFORE_UPDATE_CATALOG_ROLE, AFTER_UPDATE_CATALOG_ROLE, BEFORE_LIST_CATALOG_ROLES, AFTER_LIST_CATALOG_ROLES, BEFORE_ADD_GRANT_TO_CATALOG_ROLE, AFTER_ADD_GRANT_TO_CATALOG_ROLE, BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE, AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE, BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE, AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE, BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE, AFTER_LIST_GRANTS_FOR_CATALOG_ROLE, BEFORE_CREATE_PRINCIPAL, AFTER_CREATE_PRINCIPAL, BEFORE_DELETE_PRINCIPAL, AFTER_DELETE_PRINCIPAL, BEFORE_GET_PRIN
CIPAL, AFTER_GET_PRINCIPAL, BEFORE_UPDATE_PRINCIPAL, AFTER_UPDATE_PRINCIPAL, BEFORE_ROTATE_CREDENTIALS, AFTER_ROTATE_CREDENTIALS, BEFORE_RESET_CREDENTIALS, AFTER_RESET_CREDENTIALS, BEFORE_LIST_PRINCIPALS, AFTER_LIST_PRINCIPALS, BEFORE_ASSIGN_PRINCIPAL_ROLE, AFTER_ASSIGN_PRINCIPAL_ROLE, BEFORE_REVOKE_PRINCIPAL_ROLE, AFTER_REVOKE_PRINCIPAL_ROLE, BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES, AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES, BEFORE_CREATE_PRINCIPAL_ROLE, AFTER_CREATE_PRINCIPAL_ROLE, BEFORE_DELETE_PRINCIPAL_ROLE, AFTER_DELETE_PRINCIPAL_ROLE, BEFORE_GET_PRINCIPAL_ROLE, AFTER_GET_PRINCIPAL_ROLE, BEFORE_UPDATE_PRINCIPAL_ROLE, AFTER_UPDATE_PRINCIPAL_ROLE, BEFORE_LIST_PRINCIPAL_ROLES, AFTER_LIST_PRINCIPAL_ROLES, BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE, AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE, BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE, AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE, BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE, AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE,
BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE, AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE, BEFORE_CREATE_NAMESPACE, AFTER_CREATE_NAMESPACE, BEFORE_LIST_NAMESPACES, AFTER_LIST_NAMESPACES, BEFORE_LOAD_NAMESPACE_METADATA, AFTER_LOAD_NAMESPACE_METADATA, BEFORE_CHECK_EXISTS_NAMESPACE, AFTER_CHECK_EXISTS_NAMESPACE, BEFORE_DROP_NAMESPACE, AFTER_DROP_NAMESPACE, BEFORE_UPDATE_NAMESPACE_PROPERTIES, AFTER_UPDATE_NAMESPACE_PROPERTIES, BEFORE_CREATE_TABLE, AFTER_CREATE_TABLE, BEFORE_LIST_TABLES, AFTER_LIST_TABLES, BEFORE_LOAD_TABLE, AFTER_LOAD_TABLE, BEFORE_CHECK_EXISTS_TABLE, AFTER_CHECK_EXISTS_TABLE, BEFORE_DROP_TABLE, AFTER_DROP_TABLE, BEFORE_REGISTER_TABLE, AFTER_REGISTER_TABLE, BEFORE_RENAME_TABLE, AFTER_RENAME_TABLE, BEFORE_UPDATE_TABLE, AFTER_UPDATE_TABLE, BEFORE_REFRESH_TABLE, AFTER_REFRESH_TABLE, BEFORE_CREATE_VIEW, AFTER_CREATE_VIEW, BEFORE_LIST_VIEWS, AFTER_LIST_VIEWS, BEFORE_LOAD_VIEW, AFTER_LOAD_VIEW, BEFORE_CHECK_EXISTS_VIEW, AFTER_CHECK_EXISTS_VIEW, BEFORE_DROP_VIEW, AFTER_DROP_VI
EW, BEFORE_RENAME_VIEW, AFTER_RENAME_VIEW, BEFORE_REPLACE_VIEW, AFTER_REPLACE_VIEW, BEFORE_REFRESH_VIEW, AFTER_REFRESH_VIEW, BEFORE_LOAD_CREDENTIALS, AFTER_LOAD_CREDENTIALS, BEFORE_COMMIT_TRANSACTION, AFTER_COMMIT_TRANSACTION, BEFORE_SEND_NOTIFICATION, AFTER_SEND_NOTIFICATION, BEFORE_GET_CONFIG, AFTER_GET_CONFIG, BEFORE_CREATE_POLICY, AFTER_CREATE_POLICY, BEFORE_LIST_POLICIES, AFTER_LIST_POLICIES, BEFORE_LOAD_POLICY, AFTER_LOAD_POLICY, BEFORE_UPDATE_POLICY, AFTER_UPDATE_POLICY, BEFORE_DROP_POLICY, AFTER_DROP_POLICY, BEFORE_ATTACH_POLICY, AFTER_ATTACH_POLICY, BEFORE_DETACH_POLICY, AFTER_DETACH_POLICY, BEFORE_GET_APPLICABLE_POLICIES, AFTER_GET_APPLICABLE_POLICIES, BEFORE_CREATE_GENERIC_TABLE, AFTER_CREATE_GENERIC_TABLE, BEFORE_DROP_GENERIC_TABLE, AFTER_DROP_GENERIC_TABLE, BEFORE_LIST_GENERIC_TABLES, AFTER_LIST_GENERIC_TABLES, BEFORE_LOAD_GENERIC_TABLE, AFTER_LOAD_GENERIC_TABLE, BEFORE_ATTEMPT_TASK, AFTER_ATTEMPT_TASK, BEFORE_LIMIT_REQUEST_RATE` | Comma separated list of enabled event
types. This event listener will only receive events of the selected types. |
Review Comment:
Good idea π
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3029096621
##########
runtime/service/src/main/java/org/apache/polaris/service/events/jsonEventListener/aws/cloudwatch/AwsCloudWatchEventListener.java:
##########
@@ -146,9 +145,6 @@ void shutdown() {
@Override
public void onEvent(PolarisEvent event) {
- if (event.type() != PolarisEventType.AFTER_REFRESH_TABLE) {
Review Comment:
I removed this, because it should be part of the configuration for cloud watch listener. To remain compatible, we could consider defaulting `AFTER_REFRESH_TABLE` as the only event type for AWS cloudwatch in the application config.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4251211981
Added a commit (13335ff0ef1cc3d6d1d83938baa280302433931f) which removes `If a listener configuration exists, events are disabled by default (unless explicitly listed in either enabled-event-types or enabled-event-categories).` sentence from the doc, since after 99a6c47de939053b689ac6598acb542fbc337ea6 it is no longer correct. Also, mentioned the new event configs in the changelog. cc @dimas-b @adutra @adnanhemani
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4239049246
> @nandorKollar : from CI: `ERROR: Configuration reference is out of date. Please run './gradlew :polaris-config-docs-site:copyConfigSectionsToSite' and commit the changes.`
The only doc change was this one: https://github.com/apache/polaris/pull/4064/changes/cb31ed9f073b2843a9cd6d5b0089f03fac841c31
`./gradlew :polaris-config-docs-site:copyConfigSectionsToSite` doesn't add any new changes.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3046909560
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Thanks Alex for the helping out with a detailed explanation. I think that the extra if statements don't add much complexity. My only concern is what I mentioned before: one can easily forget wrapping the dispatch calls around. Also, I don't think we can easily verify, that anytime one calls dispatch, it is wrapped around with that if statement. Nonetheless, I also vote keeping this approach.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3071619992
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
- Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes =
+ listenerConfiguration == null
Review Comment:
Despite this doesn't seem to be a problem now, it is reasonable to expect that when someone extends `ListenerConfiguration`, then there should be no need to list every event type. How about this:
```
var supportedTypes =
listenerConfiguration == null && !listenerConfiguration.enabledEventCategories().isPresent() && !listenerConfiguration.enabledEventTypes().isPresent()
? EnumSet.allOf(PolarisEventType.class)
: EnumSet.noneOf(PolarisEventType.class);
```
This should make it future-proof though I don't think we can cover this with a test case, as `ListenerConfiguration` has only these two configs now.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3032844881
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
I thought about AOP too but as you pointed out, the way we accumulate event attributes gets in the way.
I think the if blocks are the least worst solution. If we are concerned about missing new event types, we probably could introduce a test that verifies that every event type is correctly received by a listener accepting all event types.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3032639218
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Ideally, we should use AOP for events, with Quarkus `@AroundInvoke` and introducing an `@PolarisEvent` annotation we could make this simple and clean. The problem is that I couldn't find a way to pass the event attribute map.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3071489359
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Thanks Dmitri, it is never too late to get to the party, as long as it still going on! :)
Do you happen to have any example for such a Quarkus plugin?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3051853580
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ Arrays.stream(PolarisEventType.values())
+ .filter(e -> e.category() == PolarisEventType.Category.CATALOG)
+ .collect(Collectors.toSet());
+
+ static final CountDownLatch latch = new CountDownLatch(5 + 3 * CATALOG_EVENTS.size());
Review Comment:
We already have Awaitility! See here for an example:
https://github.com/apache/polaris/blob/eb28eda6cdde1d7c24280112eef108f3a367bc6c/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java#L606-L626
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4241364991
Oh one more thing: should I also mention this change in the changelog?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3076935876
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
- Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes =
+ listenerConfiguration == null
Review Comment:
I think what @nandorKollar got the logic right on this comment - thanks!
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java:
##########
@@ -19,189 +19,223 @@
package org.apache.polaris.service.events;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
public enum PolarisEventType {
// Catalog Events
- BEFORE_CREATE_CATALOG(100),
- AFTER_CREATE_CATALOG(101),
- BEFORE_DELETE_CATALOG(102),
- AFTER_DELETE_CATALOG(103),
- BEFORE_GET_CATALOG(104),
- AFTER_GET_CATALOG(105),
- BEFORE_UPDATE_CATALOG(106),
- AFTER_UPDATE_CATALOG(107),
- BEFORE_LIST_CATALOGS(108),
- AFTER_LIST_CATALOGS(109),
+ BEFORE_CREATE_CATALOG(100, Category.CATALOG),
+ AFTER_CREATE_CATALOG(101, Category.CATALOG),
+ BEFORE_DELETE_CATALOG(102, Category.CATALOG),
+ AFTER_DELETE_CATALOG(103, Category.CATALOG),
+ BEFORE_GET_CATALOG(104, Category.CATALOG),
+ AFTER_GET_CATALOG(105, Category.CATALOG),
+ BEFORE_UPDATE_CATALOG(106, Category.CATALOG),
+ AFTER_UPDATE_CATALOG(107, Category.CATALOG),
+ BEFORE_LIST_CATALOGS(108, Category.CATALOG),
+ AFTER_LIST_CATALOGS(109, Category.CATALOG),
// Catalog Role Events
- BEFORE_CREATE_CATALOG_ROLE(200),
- AFTER_CREATE_CATALOG_ROLE(201),
- BEFORE_DELETE_CATALOG_ROLE(202),
- AFTER_DELETE_CATALOG_ROLE(203),
- BEFORE_GET_CATALOG_ROLE(204),
- AFTER_GET_CATALOG_ROLE(205),
- BEFORE_UPDATE_CATALOG_ROLE(206),
- AFTER_UPDATE_CATALOG_ROLE(207),
- BEFORE_LIST_CATALOG_ROLES(208),
- AFTER_LIST_CATALOG_ROLES(209),
- BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210),
- AFTER_ADD_GRANT_TO_CATALOG_ROLE(211),
- BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212),
- AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213),
- BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214),
- AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215),
- BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216),
- AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217),
+ BEFORE_CREATE_CATALOG_ROLE(200, Category.CATALOG_ROLE),
+ AFTER_CREATE_CATALOG_ROLE(201, Category.CATALOG_ROLE),
+ BEFORE_DELETE_CATALOG_ROLE(202, Category.CATALOG_ROLE),
+ AFTER_DELETE_CATALOG_ROLE(203, Category.CATALOG_ROLE),
+ BEFORE_GET_CATALOG_ROLE(204, Category.CATALOG_ROLE),
+ AFTER_GET_CATALOG_ROLE(205, Category.CATALOG_ROLE),
+ BEFORE_UPDATE_CATALOG_ROLE(206, Category.CATALOG_ROLE),
+ AFTER_UPDATE_CATALOG_ROLE(207, Category.CATALOG_ROLE),
+ BEFORE_LIST_CATALOG_ROLES(208, Category.CATALOG_ROLE),
+ AFTER_LIST_CATALOG_ROLES(209, Category.CATALOG_ROLE),
+ BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210, Category.CATALOG_ROLE),
+ AFTER_ADD_GRANT_TO_CATALOG_ROLE(211, Category.CATALOG_ROLE),
+ BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212, Category.CATALOG_ROLE),
+ AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213, Category.CATALOG_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214, Category.CATALOG_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215, Category.CATALOG_ROLE),
+ BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216, Category.CATALOG_ROLE),
+ AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217, Category.CATALOG_ROLE),
// Principal Events
- BEFORE_CREATE_PRINCIPAL(300),
- AFTER_CREATE_PRINCIPAL(301),
- BEFORE_DELETE_PRINCIPAL(302),
- AFTER_DELETE_PRINCIPAL(303),
- BEFORE_GET_PRINCIPAL(304),
- AFTER_GET_PRINCIPAL(305),
- BEFORE_UPDATE_PRINCIPAL(306),
- AFTER_UPDATE_PRINCIPAL(307),
- BEFORE_ROTATE_CREDENTIALS(308),
- AFTER_ROTATE_CREDENTIALS(309),
- BEFORE_RESET_CREDENTIALS(310),
- AFTER_RESET_CREDENTIALS(311),
- BEFORE_LIST_PRINCIPALS(312),
- AFTER_LIST_PRINCIPALS(313),
- BEFORE_ASSIGN_PRINCIPAL_ROLE(314),
- AFTER_ASSIGN_PRINCIPAL_ROLE(315),
- BEFORE_REVOKE_PRINCIPAL_ROLE(316),
- AFTER_REVOKE_PRINCIPAL_ROLE(317),
- BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318),
- AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319),
+ BEFORE_CREATE_PRINCIPAL(300, Category.PRINCIPAL),
+ AFTER_CREATE_PRINCIPAL(301, Category.PRINCIPAL),
+ BEFORE_DELETE_PRINCIPAL(302, Category.PRINCIPAL),
+ AFTER_DELETE_PRINCIPAL(303, Category.PRINCIPAL),
+ BEFORE_GET_PRINCIPAL(304, Category.PRINCIPAL),
+ AFTER_GET_PRINCIPAL(305, Category.PRINCIPAL),
+ BEFORE_UPDATE_PRINCIPAL(306, Category.PRINCIPAL),
+ AFTER_UPDATE_PRINCIPAL(307, Category.PRINCIPAL),
+ BEFORE_ROTATE_CREDENTIALS(308, Category.PRINCIPAL),
+ AFTER_ROTATE_CREDENTIALS(309, Category.PRINCIPAL),
+ BEFORE_RESET_CREDENTIALS(310, Category.PRINCIPAL),
+ AFTER_RESET_CREDENTIALS(311, Category.PRINCIPAL),
+ BEFORE_LIST_PRINCIPALS(312, Category.PRINCIPAL),
+ AFTER_LIST_PRINCIPALS(313, Category.PRINCIPAL),
+ BEFORE_ASSIGN_PRINCIPAL_ROLE(314, Category.PRINCIPAL),
+ AFTER_ASSIGN_PRINCIPAL_ROLE(315, Category.PRINCIPAL),
+ BEFORE_REVOKE_PRINCIPAL_ROLE(316, Category.PRINCIPAL),
+ AFTER_REVOKE_PRINCIPAL_ROLE(317, Category.PRINCIPAL),
+ BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318, Category.PRINCIPAL),
+ AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319, Category.PRINCIPAL),
// Principal Role Events
- BEFORE_CREATE_PRINCIPAL_ROLE(400),
- AFTER_CREATE_PRINCIPAL_ROLE(401),
- BEFORE_DELETE_PRINCIPAL_ROLE(402),
- AFTER_DELETE_PRINCIPAL_ROLE(403),
- BEFORE_GET_PRINCIPAL_ROLE(404),
- AFTER_GET_PRINCIPAL_ROLE(405),
- BEFORE_UPDATE_PRINCIPAL_ROLE(406),
- AFTER_UPDATE_PRINCIPAL_ROLE(407),
- BEFORE_LIST_PRINCIPAL_ROLES(408),
- AFTER_LIST_PRINCIPAL_ROLES(409),
- BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410),
- AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411),
- BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412),
- AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413),
- BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414),
- AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415),
- BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416),
- AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417),
+ BEFORE_CREATE_PRINCIPAL_ROLE(400, Category.PRINCIPAL_ROLE),
+ AFTER_CREATE_PRINCIPAL_ROLE(401, Category.PRINCIPAL_ROLE),
+ BEFORE_DELETE_PRINCIPAL_ROLE(402, Category.PRINCIPAL_ROLE),
+ AFTER_DELETE_PRINCIPAL_ROLE(403, Category.PRINCIPAL_ROLE),
+ BEFORE_GET_PRINCIPAL_ROLE(404, Category.PRINCIPAL_ROLE),
+ AFTER_GET_PRINCIPAL_ROLE(405, Category.PRINCIPAL_ROLE),
+ BEFORE_UPDATE_PRINCIPAL_ROLE(406, Category.PRINCIPAL_ROLE),
+ AFTER_UPDATE_PRINCIPAL_ROLE(407, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_PRINCIPAL_ROLES(408, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_PRINCIPAL_ROLES(409, Category.PRINCIPAL_ROLE),
+ BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410, Category.PRINCIPAL_ROLE),
+ AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411, Category.PRINCIPAL_ROLE),
+ BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412, Category.PRINCIPAL_ROLE),
+ AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417, Category.PRINCIPAL_ROLE),
// Namespace Events
- BEFORE_CREATE_NAMESPACE(500),
- AFTER_CREATE_NAMESPACE(501),
- BEFORE_LIST_NAMESPACES(502),
- AFTER_LIST_NAMESPACES(503),
- BEFORE_LOAD_NAMESPACE_METADATA(504),
- AFTER_LOAD_NAMESPACE_METADATA(505),
- BEFORE_CHECK_EXISTS_NAMESPACE(506),
- AFTER_CHECK_EXISTS_NAMESPACE(507),
- BEFORE_DROP_NAMESPACE(508),
- AFTER_DROP_NAMESPACE(509),
- BEFORE_UPDATE_NAMESPACE_PROPERTIES(510),
- AFTER_UPDATE_NAMESPACE_PROPERTIES(511),
+ BEFORE_CREATE_NAMESPACE(500, Category.NAMESPACE),
+ AFTER_CREATE_NAMESPACE(501, Category.NAMESPACE),
+ BEFORE_LIST_NAMESPACES(502, Category.NAMESPACE),
+ AFTER_LIST_NAMESPACES(503, Category.NAMESPACE),
+ BEFORE_LOAD_NAMESPACE_METADATA(504, Category.NAMESPACE),
+ AFTER_LOAD_NAMESPACE_METADATA(505, Category.NAMESPACE),
+ BEFORE_CHECK_EXISTS_NAMESPACE(506, Category.NAMESPACE),
+ AFTER_CHECK_EXISTS_NAMESPACE(507, Category.NAMESPACE),
+ BEFORE_DROP_NAMESPACE(508, Category.NAMESPACE),
+ AFTER_DROP_NAMESPACE(509, Category.NAMESPACE),
+ BEFORE_UPDATE_NAMESPACE_PROPERTIES(510, Category.NAMESPACE),
+ AFTER_UPDATE_NAMESPACE_PROPERTIES(511, Category.NAMESPACE),
// Table Events
- BEFORE_CREATE_TABLE(600),
- AFTER_CREATE_TABLE(601),
- BEFORE_LIST_TABLES(602),
- AFTER_LIST_TABLES(603),
- BEFORE_LOAD_TABLE(604),
- AFTER_LOAD_TABLE(605),
- BEFORE_CHECK_EXISTS_TABLE(606),
- AFTER_CHECK_EXISTS_TABLE(607),
- BEFORE_DROP_TABLE(608),
- AFTER_DROP_TABLE(609),
- BEFORE_REGISTER_TABLE(610),
- AFTER_REGISTER_TABLE(611),
- BEFORE_RENAME_TABLE(612),
- AFTER_RENAME_TABLE(613),
- BEFORE_UPDATE_TABLE(614),
- AFTER_UPDATE_TABLE(615),
- BEFORE_REFRESH_TABLE(616),
- AFTER_REFRESH_TABLE(617),
+ BEFORE_CREATE_TABLE(600, Category.TABLE),
+ AFTER_CREATE_TABLE(601, Category.TABLE),
+ BEFORE_LIST_TABLES(602, Category.TABLE),
+ AFTER_LIST_TABLES(603, Category.TABLE),
+ BEFORE_LOAD_TABLE(604, Category.TABLE),
+ AFTER_LOAD_TABLE(605, Category.TABLE),
+ BEFORE_CHECK_EXISTS_TABLE(606, Category.TABLE),
+ AFTER_CHECK_EXISTS_TABLE(607, Category.TABLE),
+ BEFORE_DROP_TABLE(608, Category.TABLE),
+ AFTER_DROP_TABLE(609, Category.TABLE),
+ BEFORE_REGISTER_TABLE(610, Category.TABLE),
+ AFTER_REGISTER_TABLE(611, Category.TABLE),
+ BEFORE_RENAME_TABLE(612, Category.TABLE),
+ AFTER_RENAME_TABLE(613, Category.TABLE),
+ BEFORE_UPDATE_TABLE(614, Category.TABLE),
+ AFTER_UPDATE_TABLE(615, Category.TABLE),
+ BEFORE_REFRESH_TABLE(616, Category.TABLE),
+ AFTER_REFRESH_TABLE(617, Category.TABLE),
// View Events
- BEFORE_CREATE_VIEW(700),
- AFTER_CREATE_VIEW(701),
- BEFORE_LIST_VIEWS(702),
- AFTER_LIST_VIEWS(703),
- BEFORE_LOAD_VIEW(704),
- AFTER_LOAD_VIEW(705),
- BEFORE_CHECK_EXISTS_VIEW(706),
- AFTER_CHECK_EXISTS_VIEW(707),
- BEFORE_DROP_VIEW(708),
- AFTER_DROP_VIEW(709),
- BEFORE_RENAME_VIEW(710),
- AFTER_RENAME_VIEW(711),
- BEFORE_REPLACE_VIEW(712),
- AFTER_REPLACE_VIEW(713),
- BEFORE_REFRESH_VIEW(714),
- AFTER_REFRESH_VIEW(715),
+ BEFORE_CREATE_VIEW(700, Category.VIEW),
+ AFTER_CREATE_VIEW(701, Category.VIEW),
+ BEFORE_LIST_VIEWS(702, Category.VIEW),
+ AFTER_LIST_VIEWS(703, Category.VIEW),
+ BEFORE_LOAD_VIEW(704, Category.VIEW),
+ AFTER_LOAD_VIEW(705, Category.VIEW),
+ BEFORE_CHECK_EXISTS_VIEW(706, Category.VIEW),
+ AFTER_CHECK_EXISTS_VIEW(707, Category.VIEW),
+ BEFORE_DROP_VIEW(708, Category.VIEW),
+ AFTER_DROP_VIEW(709, Category.VIEW),
+ BEFORE_RENAME_VIEW(710, Category.VIEW),
+ AFTER_RENAME_VIEW(711, Category.VIEW),
+ BEFORE_REPLACE_VIEW(712, Category.VIEW),
+ AFTER_REPLACE_VIEW(713, Category.VIEW),
+ BEFORE_REFRESH_VIEW(714, Category.VIEW),
+ AFTER_REFRESH_VIEW(715, Category.VIEW),
// Credential Events
- BEFORE_LOAD_CREDENTIALS(800),
- AFTER_LOAD_CREDENTIALS(801),
+ BEFORE_LOAD_CREDENTIALS(800, Category.CREDENTIAL),
+ AFTER_LOAD_CREDENTIALS(801, Category.CREDENTIAL),
// Transaction Events
- BEFORE_COMMIT_TRANSACTION(900),
- AFTER_COMMIT_TRANSACTION(901),
+ BEFORE_COMMIT_TRANSACTION(900, Category.TRANSACTION),
+ AFTER_COMMIT_TRANSACTION(901, Category.TRANSACTION),
// Notification Events
- BEFORE_SEND_NOTIFICATION(1000),
- AFTER_SEND_NOTIFICATION(1001),
+ BEFORE_SEND_NOTIFICATION(1000, Category.NOTIFICATION),
+ AFTER_SEND_NOTIFICATION(1001, Category.NOTIFICATION),
// Configuration Events
- BEFORE_GET_CONFIG(1100),
- AFTER_GET_CONFIG(1101),
+ BEFORE_GET_CONFIG(1100, Category.CONFIG),
+ AFTER_GET_CONFIG(1101, Category.CONFIG),
// Policy Events
- BEFORE_CREATE_POLICY(1200),
- AFTER_CREATE_POLICY(1201),
- BEFORE_LIST_POLICIES(1202),
- AFTER_LIST_POLICIES(1203),
- BEFORE_LOAD_POLICY(1204),
- AFTER_LOAD_POLICY(1205),
- BEFORE_UPDATE_POLICY(1206),
- AFTER_UPDATE_POLICY(1207),
- BEFORE_DROP_POLICY(1208),
- AFTER_DROP_POLICY(1209),
- BEFORE_ATTACH_POLICY(1210),
- AFTER_ATTACH_POLICY(1211),
- BEFORE_DETACH_POLICY(1212),
- AFTER_DETACH_POLICY(1213),
- BEFORE_GET_APPLICABLE_POLICIES(1214),
- AFTER_GET_APPLICABLE_POLICIES(1215),
+ BEFORE_CREATE_POLICY(1200, Category.POLICY),
+ AFTER_CREATE_POLICY(1201, Category.POLICY),
+ BEFORE_LIST_POLICIES(1202, Category.POLICY),
+ AFTER_LIST_POLICIES(1203, Category.POLICY),
+ BEFORE_LOAD_POLICY(1204, Category.POLICY),
+ AFTER_LOAD_POLICY(1205, Category.POLICY),
+ BEFORE_UPDATE_POLICY(1206, Category.POLICY),
+ AFTER_UPDATE_POLICY(1207, Category.POLICY),
+ BEFORE_DROP_POLICY(1208, Category.POLICY),
+ AFTER_DROP_POLICY(1209, Category.POLICY),
+ BEFORE_ATTACH_POLICY(1210, Category.POLICY),
+ AFTER_ATTACH_POLICY(1211, Category.POLICY),
+ BEFORE_DETACH_POLICY(1212, Category.POLICY),
+ AFTER_DETACH_POLICY(1213, Category.POLICY),
+ BEFORE_GET_APPLICABLE_POLICIES(1214, Category.POLICY),
+ AFTER_GET_APPLICABLE_POLICIES(1215, Category.POLICY),
// Generic Table Events
- BEFORE_CREATE_GENERIC_TABLE(1300),
- AFTER_CREATE_GENERIC_TABLE(1301),
- BEFORE_DROP_GENERIC_TABLE(1302),
- AFTER_DROP_GENERIC_TABLE(1303),
- BEFORE_LIST_GENERIC_TABLES(1304),
- AFTER_LIST_GENERIC_TABLES(1305),
- BEFORE_LOAD_GENERIC_TABLE(1306),
- AFTER_LOAD_GENERIC_TABLE(1307),
+ BEFORE_CREATE_GENERIC_TABLE(1300, Category.GENERIC_TABLE),
+ AFTER_CREATE_GENERIC_TABLE(1301, Category.GENERIC_TABLE),
+ BEFORE_DROP_GENERIC_TABLE(1302, Category.GENERIC_TABLE),
+ AFTER_DROP_GENERIC_TABLE(1303, Category.GENERIC_TABLE),
+ BEFORE_LIST_GENERIC_TABLES(1304, Category.GENERIC_TABLE),
+ AFTER_LIST_GENERIC_TABLES(1305, Category.GENERIC_TABLE),
+ BEFORE_LOAD_GENERIC_TABLE(1306, Category.GENERIC_TABLE),
+ AFTER_LOAD_GENERIC_TABLE(1307, Category.GENERIC_TABLE),
// Task Execution Events
- BEFORE_ATTEMPT_TASK(1400),
- AFTER_ATTEMPT_TASK(1401),
+ BEFORE_ATTEMPT_TASK(1400, Category.TASK_EXECUTION),
+ AFTER_ATTEMPT_TASK(1401, Category.TASK_EXECUTION),
// Rate Limiting Events
- BEFORE_LIMIT_REQUEST_RATE(1500);
+ BEFORE_LIMIT_REQUEST_RATE(1500, Category.RATE_LIMITING);
private final int code;
+ private final Category category;
- private PolarisEventType(int code) {
+ PolarisEventType(int code, Category category) {
this.code = code;
+ this.category = category;
}
public int code() {
return code;
}
+
+ public Category category() {
+ return category;
+ }
+
+ public static Set<PolarisEventType> typesOfCategory(Category category) {
Review Comment:
Oh, my bad - I misread this, I thought it was being used in the request path rather than just startup. This is fine with me then.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4181042063
@nandorKollar : since this PR builds on #3442 , would you mind tagging @olsoloviov as a co-author (in the PR description to be later propagated to the commit message)?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3032865488
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
That's good points, indeed. I agree with @adutra , the "if" chain is probably the least worst.
Thanks !
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3067157529
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Sorry for coming later to this party π
Using AOP is very interesting in this case. However, rather that using it on the leaf call to dispatch an event, we could generate the whole interceptor / delegator class (probably by making a Quarkus plugin) and only call the event-producing code when a listener is present.
This would way outside the scope of current PR, though :)
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4281692318
@nandorKollar do you mind fixing the conflicts? After that I'll merge immediately. Thanks π
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3080973861
##########
runtime/service/src/main/java/org/apache/polaris/service/events/jsonEventListener/aws/cloudwatch/AwsCloudWatchEventListener.java:
##########
@@ -146,9 +145,6 @@ void shutdown() {
@Override
public void onEvent(PolarisEvent event) {
- if (event.type() != PolarisEventType.AFTER_REFRESH_TABLE) {
Review Comment:
Delegating this check to configuration sounds reasonable to me... it's probably worth mentioning as a (non-breaking) "Change" in CHANGELOG.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3074559703
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java:
##########
@@ -19,189 +19,223 @@
package org.apache.polaris.service.events;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
public enum PolarisEventType {
// Catalog Events
- BEFORE_CREATE_CATALOG(100),
- AFTER_CREATE_CATALOG(101),
- BEFORE_DELETE_CATALOG(102),
- AFTER_DELETE_CATALOG(103),
- BEFORE_GET_CATALOG(104),
- AFTER_GET_CATALOG(105),
- BEFORE_UPDATE_CATALOG(106),
- AFTER_UPDATE_CATALOG(107),
- BEFORE_LIST_CATALOGS(108),
- AFTER_LIST_CATALOGS(109),
+ BEFORE_CREATE_CATALOG(100, Category.CATALOG),
+ AFTER_CREATE_CATALOG(101, Category.CATALOG),
+ BEFORE_DELETE_CATALOG(102, Category.CATALOG),
+ AFTER_DELETE_CATALOG(103, Category.CATALOG),
+ BEFORE_GET_CATALOG(104, Category.CATALOG),
+ AFTER_GET_CATALOG(105, Category.CATALOG),
+ BEFORE_UPDATE_CATALOG(106, Category.CATALOG),
+ AFTER_UPDATE_CATALOG(107, Category.CATALOG),
+ BEFORE_LIST_CATALOGS(108, Category.CATALOG),
+ AFTER_LIST_CATALOGS(109, Category.CATALOG),
// Catalog Role Events
- BEFORE_CREATE_CATALOG_ROLE(200),
- AFTER_CREATE_CATALOG_ROLE(201),
- BEFORE_DELETE_CATALOG_ROLE(202),
- AFTER_DELETE_CATALOG_ROLE(203),
- BEFORE_GET_CATALOG_ROLE(204),
- AFTER_GET_CATALOG_ROLE(205),
- BEFORE_UPDATE_CATALOG_ROLE(206),
- AFTER_UPDATE_CATALOG_ROLE(207),
- BEFORE_LIST_CATALOG_ROLES(208),
- AFTER_LIST_CATALOG_ROLES(209),
- BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210),
- AFTER_ADD_GRANT_TO_CATALOG_ROLE(211),
- BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212),
- AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213),
- BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214),
- AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215),
- BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216),
- AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217),
+ BEFORE_CREATE_CATALOG_ROLE(200, Category.CATALOG_ROLE),
+ AFTER_CREATE_CATALOG_ROLE(201, Category.CATALOG_ROLE),
+ BEFORE_DELETE_CATALOG_ROLE(202, Category.CATALOG_ROLE),
+ AFTER_DELETE_CATALOG_ROLE(203, Category.CATALOG_ROLE),
+ BEFORE_GET_CATALOG_ROLE(204, Category.CATALOG_ROLE),
+ AFTER_GET_CATALOG_ROLE(205, Category.CATALOG_ROLE),
+ BEFORE_UPDATE_CATALOG_ROLE(206, Category.CATALOG_ROLE),
+ AFTER_UPDATE_CATALOG_ROLE(207, Category.CATALOG_ROLE),
+ BEFORE_LIST_CATALOG_ROLES(208, Category.CATALOG_ROLE),
+ AFTER_LIST_CATALOG_ROLES(209, Category.CATALOG_ROLE),
+ BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210, Category.CATALOG_ROLE),
+ AFTER_ADD_GRANT_TO_CATALOG_ROLE(211, Category.CATALOG_ROLE),
+ BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212, Category.CATALOG_ROLE),
+ AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213, Category.CATALOG_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214, Category.CATALOG_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215, Category.CATALOG_ROLE),
+ BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216, Category.CATALOG_ROLE),
+ AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217, Category.CATALOG_ROLE),
// Principal Events
- BEFORE_CREATE_PRINCIPAL(300),
- AFTER_CREATE_PRINCIPAL(301),
- BEFORE_DELETE_PRINCIPAL(302),
- AFTER_DELETE_PRINCIPAL(303),
- BEFORE_GET_PRINCIPAL(304),
- AFTER_GET_PRINCIPAL(305),
- BEFORE_UPDATE_PRINCIPAL(306),
- AFTER_UPDATE_PRINCIPAL(307),
- BEFORE_ROTATE_CREDENTIALS(308),
- AFTER_ROTATE_CREDENTIALS(309),
- BEFORE_RESET_CREDENTIALS(310),
- AFTER_RESET_CREDENTIALS(311),
- BEFORE_LIST_PRINCIPALS(312),
- AFTER_LIST_PRINCIPALS(313),
- BEFORE_ASSIGN_PRINCIPAL_ROLE(314),
- AFTER_ASSIGN_PRINCIPAL_ROLE(315),
- BEFORE_REVOKE_PRINCIPAL_ROLE(316),
- AFTER_REVOKE_PRINCIPAL_ROLE(317),
- BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318),
- AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319),
+ BEFORE_CREATE_PRINCIPAL(300, Category.PRINCIPAL),
+ AFTER_CREATE_PRINCIPAL(301, Category.PRINCIPAL),
+ BEFORE_DELETE_PRINCIPAL(302, Category.PRINCIPAL),
+ AFTER_DELETE_PRINCIPAL(303, Category.PRINCIPAL),
+ BEFORE_GET_PRINCIPAL(304, Category.PRINCIPAL),
+ AFTER_GET_PRINCIPAL(305, Category.PRINCIPAL),
+ BEFORE_UPDATE_PRINCIPAL(306, Category.PRINCIPAL),
+ AFTER_UPDATE_PRINCIPAL(307, Category.PRINCIPAL),
+ BEFORE_ROTATE_CREDENTIALS(308, Category.PRINCIPAL),
+ AFTER_ROTATE_CREDENTIALS(309, Category.PRINCIPAL),
+ BEFORE_RESET_CREDENTIALS(310, Category.PRINCIPAL),
+ AFTER_RESET_CREDENTIALS(311, Category.PRINCIPAL),
+ BEFORE_LIST_PRINCIPALS(312, Category.PRINCIPAL),
+ AFTER_LIST_PRINCIPALS(313, Category.PRINCIPAL),
+ BEFORE_ASSIGN_PRINCIPAL_ROLE(314, Category.PRINCIPAL),
+ AFTER_ASSIGN_PRINCIPAL_ROLE(315, Category.PRINCIPAL),
+ BEFORE_REVOKE_PRINCIPAL_ROLE(316, Category.PRINCIPAL),
+ AFTER_REVOKE_PRINCIPAL_ROLE(317, Category.PRINCIPAL),
+ BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318, Category.PRINCIPAL),
+ AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319, Category.PRINCIPAL),
// Principal Role Events
- BEFORE_CREATE_PRINCIPAL_ROLE(400),
- AFTER_CREATE_PRINCIPAL_ROLE(401),
- BEFORE_DELETE_PRINCIPAL_ROLE(402),
- AFTER_DELETE_PRINCIPAL_ROLE(403),
- BEFORE_GET_PRINCIPAL_ROLE(404),
- AFTER_GET_PRINCIPAL_ROLE(405),
- BEFORE_UPDATE_PRINCIPAL_ROLE(406),
- AFTER_UPDATE_PRINCIPAL_ROLE(407),
- BEFORE_LIST_PRINCIPAL_ROLES(408),
- AFTER_LIST_PRINCIPAL_ROLES(409),
- BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410),
- AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411),
- BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412),
- AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413),
- BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414),
- AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415),
- BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416),
- AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417),
+ BEFORE_CREATE_PRINCIPAL_ROLE(400, Category.PRINCIPAL_ROLE),
+ AFTER_CREATE_PRINCIPAL_ROLE(401, Category.PRINCIPAL_ROLE),
+ BEFORE_DELETE_PRINCIPAL_ROLE(402, Category.PRINCIPAL_ROLE),
+ AFTER_DELETE_PRINCIPAL_ROLE(403, Category.PRINCIPAL_ROLE),
+ BEFORE_GET_PRINCIPAL_ROLE(404, Category.PRINCIPAL_ROLE),
+ AFTER_GET_PRINCIPAL_ROLE(405, Category.PRINCIPAL_ROLE),
+ BEFORE_UPDATE_PRINCIPAL_ROLE(406, Category.PRINCIPAL_ROLE),
+ AFTER_UPDATE_PRINCIPAL_ROLE(407, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_PRINCIPAL_ROLES(408, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_PRINCIPAL_ROLES(409, Category.PRINCIPAL_ROLE),
+ BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410, Category.PRINCIPAL_ROLE),
+ AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411, Category.PRINCIPAL_ROLE),
+ BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412, Category.PRINCIPAL_ROLE),
+ AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417, Category.PRINCIPAL_ROLE),
// Namespace Events
- BEFORE_CREATE_NAMESPACE(500),
- AFTER_CREATE_NAMESPACE(501),
- BEFORE_LIST_NAMESPACES(502),
- AFTER_LIST_NAMESPACES(503),
- BEFORE_LOAD_NAMESPACE_METADATA(504),
- AFTER_LOAD_NAMESPACE_METADATA(505),
- BEFORE_CHECK_EXISTS_NAMESPACE(506),
- AFTER_CHECK_EXISTS_NAMESPACE(507),
- BEFORE_DROP_NAMESPACE(508),
- AFTER_DROP_NAMESPACE(509),
- BEFORE_UPDATE_NAMESPACE_PROPERTIES(510),
- AFTER_UPDATE_NAMESPACE_PROPERTIES(511),
+ BEFORE_CREATE_NAMESPACE(500, Category.NAMESPACE),
+ AFTER_CREATE_NAMESPACE(501, Category.NAMESPACE),
+ BEFORE_LIST_NAMESPACES(502, Category.NAMESPACE),
+ AFTER_LIST_NAMESPACES(503, Category.NAMESPACE),
+ BEFORE_LOAD_NAMESPACE_METADATA(504, Category.NAMESPACE),
+ AFTER_LOAD_NAMESPACE_METADATA(505, Category.NAMESPACE),
+ BEFORE_CHECK_EXISTS_NAMESPACE(506, Category.NAMESPACE),
+ AFTER_CHECK_EXISTS_NAMESPACE(507, Category.NAMESPACE),
+ BEFORE_DROP_NAMESPACE(508, Category.NAMESPACE),
+ AFTER_DROP_NAMESPACE(509, Category.NAMESPACE),
+ BEFORE_UPDATE_NAMESPACE_PROPERTIES(510, Category.NAMESPACE),
+ AFTER_UPDATE_NAMESPACE_PROPERTIES(511, Category.NAMESPACE),
// Table Events
- BEFORE_CREATE_TABLE(600),
- AFTER_CREATE_TABLE(601),
- BEFORE_LIST_TABLES(602),
- AFTER_LIST_TABLES(603),
- BEFORE_LOAD_TABLE(604),
- AFTER_LOAD_TABLE(605),
- BEFORE_CHECK_EXISTS_TABLE(606),
- AFTER_CHECK_EXISTS_TABLE(607),
- BEFORE_DROP_TABLE(608),
- AFTER_DROP_TABLE(609),
- BEFORE_REGISTER_TABLE(610),
- AFTER_REGISTER_TABLE(611),
- BEFORE_RENAME_TABLE(612),
- AFTER_RENAME_TABLE(613),
- BEFORE_UPDATE_TABLE(614),
- AFTER_UPDATE_TABLE(615),
- BEFORE_REFRESH_TABLE(616),
- AFTER_REFRESH_TABLE(617),
+ BEFORE_CREATE_TABLE(600, Category.TABLE),
+ AFTER_CREATE_TABLE(601, Category.TABLE),
+ BEFORE_LIST_TABLES(602, Category.TABLE),
+ AFTER_LIST_TABLES(603, Category.TABLE),
+ BEFORE_LOAD_TABLE(604, Category.TABLE),
+ AFTER_LOAD_TABLE(605, Category.TABLE),
+ BEFORE_CHECK_EXISTS_TABLE(606, Category.TABLE),
+ AFTER_CHECK_EXISTS_TABLE(607, Category.TABLE),
+ BEFORE_DROP_TABLE(608, Category.TABLE),
+ AFTER_DROP_TABLE(609, Category.TABLE),
+ BEFORE_REGISTER_TABLE(610, Category.TABLE),
+ AFTER_REGISTER_TABLE(611, Category.TABLE),
+ BEFORE_RENAME_TABLE(612, Category.TABLE),
+ AFTER_RENAME_TABLE(613, Category.TABLE),
+ BEFORE_UPDATE_TABLE(614, Category.TABLE),
+ AFTER_UPDATE_TABLE(615, Category.TABLE),
+ BEFORE_REFRESH_TABLE(616, Category.TABLE),
+ AFTER_REFRESH_TABLE(617, Category.TABLE),
// View Events
- BEFORE_CREATE_VIEW(700),
- AFTER_CREATE_VIEW(701),
- BEFORE_LIST_VIEWS(702),
- AFTER_LIST_VIEWS(703),
- BEFORE_LOAD_VIEW(704),
- AFTER_LOAD_VIEW(705),
- BEFORE_CHECK_EXISTS_VIEW(706),
- AFTER_CHECK_EXISTS_VIEW(707),
- BEFORE_DROP_VIEW(708),
- AFTER_DROP_VIEW(709),
- BEFORE_RENAME_VIEW(710),
- AFTER_RENAME_VIEW(711),
- BEFORE_REPLACE_VIEW(712),
- AFTER_REPLACE_VIEW(713),
- BEFORE_REFRESH_VIEW(714),
- AFTER_REFRESH_VIEW(715),
+ BEFORE_CREATE_VIEW(700, Category.VIEW),
+ AFTER_CREATE_VIEW(701, Category.VIEW),
+ BEFORE_LIST_VIEWS(702, Category.VIEW),
+ AFTER_LIST_VIEWS(703, Category.VIEW),
+ BEFORE_LOAD_VIEW(704, Category.VIEW),
+ AFTER_LOAD_VIEW(705, Category.VIEW),
+ BEFORE_CHECK_EXISTS_VIEW(706, Category.VIEW),
+ AFTER_CHECK_EXISTS_VIEW(707, Category.VIEW),
+ BEFORE_DROP_VIEW(708, Category.VIEW),
+ AFTER_DROP_VIEW(709, Category.VIEW),
+ BEFORE_RENAME_VIEW(710, Category.VIEW),
+ AFTER_RENAME_VIEW(711, Category.VIEW),
+ BEFORE_REPLACE_VIEW(712, Category.VIEW),
+ AFTER_REPLACE_VIEW(713, Category.VIEW),
+ BEFORE_REFRESH_VIEW(714, Category.VIEW),
+ AFTER_REFRESH_VIEW(715, Category.VIEW),
// Credential Events
- BEFORE_LOAD_CREDENTIALS(800),
- AFTER_LOAD_CREDENTIALS(801),
+ BEFORE_LOAD_CREDENTIALS(800, Category.CREDENTIAL),
+ AFTER_LOAD_CREDENTIALS(801, Category.CREDENTIAL),
// Transaction Events
- BEFORE_COMMIT_TRANSACTION(900),
- AFTER_COMMIT_TRANSACTION(901),
+ BEFORE_COMMIT_TRANSACTION(900, Category.TRANSACTION),
+ AFTER_COMMIT_TRANSACTION(901, Category.TRANSACTION),
// Notification Events
- BEFORE_SEND_NOTIFICATION(1000),
- AFTER_SEND_NOTIFICATION(1001),
+ BEFORE_SEND_NOTIFICATION(1000, Category.NOTIFICATION),
+ AFTER_SEND_NOTIFICATION(1001, Category.NOTIFICATION),
// Configuration Events
- BEFORE_GET_CONFIG(1100),
- AFTER_GET_CONFIG(1101),
+ BEFORE_GET_CONFIG(1100, Category.CONFIG),
+ AFTER_GET_CONFIG(1101, Category.CONFIG),
// Policy Events
- BEFORE_CREATE_POLICY(1200),
- AFTER_CREATE_POLICY(1201),
- BEFORE_LIST_POLICIES(1202),
- AFTER_LIST_POLICIES(1203),
- BEFORE_LOAD_POLICY(1204),
- AFTER_LOAD_POLICY(1205),
- BEFORE_UPDATE_POLICY(1206),
- AFTER_UPDATE_POLICY(1207),
- BEFORE_DROP_POLICY(1208),
- AFTER_DROP_POLICY(1209),
- BEFORE_ATTACH_POLICY(1210),
- AFTER_ATTACH_POLICY(1211),
- BEFORE_DETACH_POLICY(1212),
- AFTER_DETACH_POLICY(1213),
- BEFORE_GET_APPLICABLE_POLICIES(1214),
- AFTER_GET_APPLICABLE_POLICIES(1215),
+ BEFORE_CREATE_POLICY(1200, Category.POLICY),
+ AFTER_CREATE_POLICY(1201, Category.POLICY),
+ BEFORE_LIST_POLICIES(1202, Category.POLICY),
+ AFTER_LIST_POLICIES(1203, Category.POLICY),
+ BEFORE_LOAD_POLICY(1204, Category.POLICY),
+ AFTER_LOAD_POLICY(1205, Category.POLICY),
+ BEFORE_UPDATE_POLICY(1206, Category.POLICY),
+ AFTER_UPDATE_POLICY(1207, Category.POLICY),
+ BEFORE_DROP_POLICY(1208, Category.POLICY),
+ AFTER_DROP_POLICY(1209, Category.POLICY),
+ BEFORE_ATTACH_POLICY(1210, Category.POLICY),
+ AFTER_ATTACH_POLICY(1211, Category.POLICY),
+ BEFORE_DETACH_POLICY(1212, Category.POLICY),
+ AFTER_DETACH_POLICY(1213, Category.POLICY),
+ BEFORE_GET_APPLICABLE_POLICIES(1214, Category.POLICY),
+ AFTER_GET_APPLICABLE_POLICIES(1215, Category.POLICY),
// Generic Table Events
- BEFORE_CREATE_GENERIC_TABLE(1300),
- AFTER_CREATE_GENERIC_TABLE(1301),
- BEFORE_DROP_GENERIC_TABLE(1302),
- AFTER_DROP_GENERIC_TABLE(1303),
- BEFORE_LIST_GENERIC_TABLES(1304),
- AFTER_LIST_GENERIC_TABLES(1305),
- BEFORE_LOAD_GENERIC_TABLE(1306),
- AFTER_LOAD_GENERIC_TABLE(1307),
+ BEFORE_CREATE_GENERIC_TABLE(1300, Category.GENERIC_TABLE),
+ AFTER_CREATE_GENERIC_TABLE(1301, Category.GENERIC_TABLE),
+ BEFORE_DROP_GENERIC_TABLE(1302, Category.GENERIC_TABLE),
+ AFTER_DROP_GENERIC_TABLE(1303, Category.GENERIC_TABLE),
+ BEFORE_LIST_GENERIC_TABLES(1304, Category.GENERIC_TABLE),
+ AFTER_LIST_GENERIC_TABLES(1305, Category.GENERIC_TABLE),
+ BEFORE_LOAD_GENERIC_TABLE(1306, Category.GENERIC_TABLE),
+ AFTER_LOAD_GENERIC_TABLE(1307, Category.GENERIC_TABLE),
// Task Execution Events
- BEFORE_ATTEMPT_TASK(1400),
- AFTER_ATTEMPT_TASK(1401),
+ BEFORE_ATTEMPT_TASK(1400, Category.TASK_EXECUTION),
+ AFTER_ATTEMPT_TASK(1401, Category.TASK_EXECUTION),
// Rate Limiting Events
- BEFORE_LIMIT_REQUEST_RATE(1500);
+ BEFORE_LIMIT_REQUEST_RATE(1500, Category.RATE_LIMITING);
private final int code;
+ private final Category category;
- private PolarisEventType(int code) {
+ PolarisEventType(int code, Category category) {
this.code = code;
+ this.category = category;
}
public int code() {
return code;
}
+
+ public Category category() {
+ return category;
+ }
+
+ public static Set<PolarisEventType> typesOfCategory(Category category) {
Review Comment:
`typesOfCategory()` is only called on startup.... why bother with a separate pre-computation step?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r2995907291
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -41,13 +41,26 @@ public class PolarisEventListeners {
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes = PolarisEventType.values();
+ if (listenerConfiguration != null && !listenerConfiguration.enabledEventTypes().isEmpty()) {
+ supportedTypes = listenerConfiguration.enabledEventTypes().toArray(PolarisEventType[]::new);
+ }
+ for (var polarisEventType : supportedTypes) {
+ eventsByType.set(polarisEventType.ordinal());
+ }
+ var listener = eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ eventBus.localConsumer(POLARIS_EVENT_CHANNEL + "." + event, handler);
Review Comment:
Right, this is not correct, will fix it soon.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3045303245
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Some more thoughts on this:
The if-block approach was designed initially with serialization in mind. IOW, the event was created with lots of serialized attributes (either JSON or `toString()` calls - I don't recall exactly). So, creating an event could be expensive.
But now that we have the events attribute map, creating a `PolarisEvent` has become much cheaper β it's just a wrapper around a thin map of references. The if block is now avoiding roughly:
- One `PolarisEvent` object (~32 bytes)
- One `EventAttributeMap` object + its internal map (~200 bytes)
- A handful of `Map.Entry` objects for the `.put()` calls
All of these are short-lived. It's not nothing, but it's not the end of the world either.
One edge case where the guard could matter is if an attribute value is computed lazily solely for the event, e.g., a `.toString()` that creates a new object. Looking at the call sites, I don't think it's happening today (**but it could, in the future**).
The `Supplier<PolarisEvent>` approach imho adds more overhead in the happy path (when there are listeners) to save a small memory overhead in the no-listeners path. All in all, it's not a perfect solution either.
So just to summarize, I'd still stick with the if block, mostly because of the risk of having some expensive-to-compute attribute added to the event.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3071489359
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java:
##########
@@ -60,52 +60,64 @@ public class PolarisCatalogsEventServiceDelegator implements PolarisCatalogsApiS
@Override
public Response createCatalog(
CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) {
- polarisEventDispatcher.dispatch(
- new PolarisEvent(
- PolarisEventType.BEFORE_CREATE_CATALOG,
- eventMetadataFactory.create(),
- new EventAttributeMap()
- .put(EventAttributes.CATALOG_NAME, request.getCatalog().getName())));
+ if (polarisEventDispatcher.hasListeners(PolarisEventType.BEFORE_CREATE_CATALOG)) {
Review Comment:
Thanks Dmitri, it is nevet too late! :)
Do you happen to have any example for such a Quarkus plugin?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3075008043
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java:
##########
@@ -19,189 +19,223 @@
package org.apache.polaris.service.events;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
public enum PolarisEventType {
// Catalog Events
- BEFORE_CREATE_CATALOG(100),
- AFTER_CREATE_CATALOG(101),
- BEFORE_DELETE_CATALOG(102),
- AFTER_DELETE_CATALOG(103),
- BEFORE_GET_CATALOG(104),
- AFTER_GET_CATALOG(105),
- BEFORE_UPDATE_CATALOG(106),
- AFTER_UPDATE_CATALOG(107),
- BEFORE_LIST_CATALOGS(108),
- AFTER_LIST_CATALOGS(109),
+ BEFORE_CREATE_CATALOG(100, Category.CATALOG),
+ AFTER_CREATE_CATALOG(101, Category.CATALOG),
+ BEFORE_DELETE_CATALOG(102, Category.CATALOG),
+ AFTER_DELETE_CATALOG(103, Category.CATALOG),
+ BEFORE_GET_CATALOG(104, Category.CATALOG),
+ AFTER_GET_CATALOG(105, Category.CATALOG),
+ BEFORE_UPDATE_CATALOG(106, Category.CATALOG),
+ AFTER_UPDATE_CATALOG(107, Category.CATALOG),
+ BEFORE_LIST_CATALOGS(108, Category.CATALOG),
+ AFTER_LIST_CATALOGS(109, Category.CATALOG),
// Catalog Role Events
- BEFORE_CREATE_CATALOG_ROLE(200),
- AFTER_CREATE_CATALOG_ROLE(201),
- BEFORE_DELETE_CATALOG_ROLE(202),
- AFTER_DELETE_CATALOG_ROLE(203),
- BEFORE_GET_CATALOG_ROLE(204),
- AFTER_GET_CATALOG_ROLE(205),
- BEFORE_UPDATE_CATALOG_ROLE(206),
- AFTER_UPDATE_CATALOG_ROLE(207),
- BEFORE_LIST_CATALOG_ROLES(208),
- AFTER_LIST_CATALOG_ROLES(209),
- BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210),
- AFTER_ADD_GRANT_TO_CATALOG_ROLE(211),
- BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212),
- AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213),
- BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214),
- AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215),
- BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216),
- AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217),
+ BEFORE_CREATE_CATALOG_ROLE(200, Category.CATALOG_ROLE),
+ AFTER_CREATE_CATALOG_ROLE(201, Category.CATALOG_ROLE),
+ BEFORE_DELETE_CATALOG_ROLE(202, Category.CATALOG_ROLE),
+ AFTER_DELETE_CATALOG_ROLE(203, Category.CATALOG_ROLE),
+ BEFORE_GET_CATALOG_ROLE(204, Category.CATALOG_ROLE),
+ AFTER_GET_CATALOG_ROLE(205, Category.CATALOG_ROLE),
+ BEFORE_UPDATE_CATALOG_ROLE(206, Category.CATALOG_ROLE),
+ AFTER_UPDATE_CATALOG_ROLE(207, Category.CATALOG_ROLE),
+ BEFORE_LIST_CATALOG_ROLES(208, Category.CATALOG_ROLE),
+ AFTER_LIST_CATALOG_ROLES(209, Category.CATALOG_ROLE),
+ BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210, Category.CATALOG_ROLE),
+ AFTER_ADD_GRANT_TO_CATALOG_ROLE(211, Category.CATALOG_ROLE),
+ BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212, Category.CATALOG_ROLE),
+ AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213, Category.CATALOG_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214, Category.CATALOG_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215, Category.CATALOG_ROLE),
+ BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216, Category.CATALOG_ROLE),
+ AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217, Category.CATALOG_ROLE),
// Principal Events
- BEFORE_CREATE_PRINCIPAL(300),
- AFTER_CREATE_PRINCIPAL(301),
- BEFORE_DELETE_PRINCIPAL(302),
- AFTER_DELETE_PRINCIPAL(303),
- BEFORE_GET_PRINCIPAL(304),
- AFTER_GET_PRINCIPAL(305),
- BEFORE_UPDATE_PRINCIPAL(306),
- AFTER_UPDATE_PRINCIPAL(307),
- BEFORE_ROTATE_CREDENTIALS(308),
- AFTER_ROTATE_CREDENTIALS(309),
- BEFORE_RESET_CREDENTIALS(310),
- AFTER_RESET_CREDENTIALS(311),
- BEFORE_LIST_PRINCIPALS(312),
- AFTER_LIST_PRINCIPALS(313),
- BEFORE_ASSIGN_PRINCIPAL_ROLE(314),
- AFTER_ASSIGN_PRINCIPAL_ROLE(315),
- BEFORE_REVOKE_PRINCIPAL_ROLE(316),
- AFTER_REVOKE_PRINCIPAL_ROLE(317),
- BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318),
- AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319),
+ BEFORE_CREATE_PRINCIPAL(300, Category.PRINCIPAL),
+ AFTER_CREATE_PRINCIPAL(301, Category.PRINCIPAL),
+ BEFORE_DELETE_PRINCIPAL(302, Category.PRINCIPAL),
+ AFTER_DELETE_PRINCIPAL(303, Category.PRINCIPAL),
+ BEFORE_GET_PRINCIPAL(304, Category.PRINCIPAL),
+ AFTER_GET_PRINCIPAL(305, Category.PRINCIPAL),
+ BEFORE_UPDATE_PRINCIPAL(306, Category.PRINCIPAL),
+ AFTER_UPDATE_PRINCIPAL(307, Category.PRINCIPAL),
+ BEFORE_ROTATE_CREDENTIALS(308, Category.PRINCIPAL),
+ AFTER_ROTATE_CREDENTIALS(309, Category.PRINCIPAL),
+ BEFORE_RESET_CREDENTIALS(310, Category.PRINCIPAL),
+ AFTER_RESET_CREDENTIALS(311, Category.PRINCIPAL),
+ BEFORE_LIST_PRINCIPALS(312, Category.PRINCIPAL),
+ AFTER_LIST_PRINCIPALS(313, Category.PRINCIPAL),
+ BEFORE_ASSIGN_PRINCIPAL_ROLE(314, Category.PRINCIPAL),
+ AFTER_ASSIGN_PRINCIPAL_ROLE(315, Category.PRINCIPAL),
+ BEFORE_REVOKE_PRINCIPAL_ROLE(316, Category.PRINCIPAL),
+ AFTER_REVOKE_PRINCIPAL_ROLE(317, Category.PRINCIPAL),
+ BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318, Category.PRINCIPAL),
+ AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319, Category.PRINCIPAL),
// Principal Role Events
- BEFORE_CREATE_PRINCIPAL_ROLE(400),
- AFTER_CREATE_PRINCIPAL_ROLE(401),
- BEFORE_DELETE_PRINCIPAL_ROLE(402),
- AFTER_DELETE_PRINCIPAL_ROLE(403),
- BEFORE_GET_PRINCIPAL_ROLE(404),
- AFTER_GET_PRINCIPAL_ROLE(405),
- BEFORE_UPDATE_PRINCIPAL_ROLE(406),
- AFTER_UPDATE_PRINCIPAL_ROLE(407),
- BEFORE_LIST_PRINCIPAL_ROLES(408),
- AFTER_LIST_PRINCIPAL_ROLES(409),
- BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410),
- AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411),
- BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412),
- AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413),
- BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414),
- AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415),
- BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416),
- AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417),
+ BEFORE_CREATE_PRINCIPAL_ROLE(400, Category.PRINCIPAL_ROLE),
+ AFTER_CREATE_PRINCIPAL_ROLE(401, Category.PRINCIPAL_ROLE),
+ BEFORE_DELETE_PRINCIPAL_ROLE(402, Category.PRINCIPAL_ROLE),
+ AFTER_DELETE_PRINCIPAL_ROLE(403, Category.PRINCIPAL_ROLE),
+ BEFORE_GET_PRINCIPAL_ROLE(404, Category.PRINCIPAL_ROLE),
+ AFTER_GET_PRINCIPAL_ROLE(405, Category.PRINCIPAL_ROLE),
+ BEFORE_UPDATE_PRINCIPAL_ROLE(406, Category.PRINCIPAL_ROLE),
+ AFTER_UPDATE_PRINCIPAL_ROLE(407, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_PRINCIPAL_ROLES(408, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_PRINCIPAL_ROLES(409, Category.PRINCIPAL_ROLE),
+ BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410, Category.PRINCIPAL_ROLE),
+ AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411, Category.PRINCIPAL_ROLE),
+ BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412, Category.PRINCIPAL_ROLE),
+ AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417, Category.PRINCIPAL_ROLE),
// Namespace Events
- BEFORE_CREATE_NAMESPACE(500),
- AFTER_CREATE_NAMESPACE(501),
- BEFORE_LIST_NAMESPACES(502),
- AFTER_LIST_NAMESPACES(503),
- BEFORE_LOAD_NAMESPACE_METADATA(504),
- AFTER_LOAD_NAMESPACE_METADATA(505),
- BEFORE_CHECK_EXISTS_NAMESPACE(506),
- AFTER_CHECK_EXISTS_NAMESPACE(507),
- BEFORE_DROP_NAMESPACE(508),
- AFTER_DROP_NAMESPACE(509),
- BEFORE_UPDATE_NAMESPACE_PROPERTIES(510),
- AFTER_UPDATE_NAMESPACE_PROPERTIES(511),
+ BEFORE_CREATE_NAMESPACE(500, Category.NAMESPACE),
+ AFTER_CREATE_NAMESPACE(501, Category.NAMESPACE),
+ BEFORE_LIST_NAMESPACES(502, Category.NAMESPACE),
+ AFTER_LIST_NAMESPACES(503, Category.NAMESPACE),
+ BEFORE_LOAD_NAMESPACE_METADATA(504, Category.NAMESPACE),
+ AFTER_LOAD_NAMESPACE_METADATA(505, Category.NAMESPACE),
+ BEFORE_CHECK_EXISTS_NAMESPACE(506, Category.NAMESPACE),
+ AFTER_CHECK_EXISTS_NAMESPACE(507, Category.NAMESPACE),
+ BEFORE_DROP_NAMESPACE(508, Category.NAMESPACE),
+ AFTER_DROP_NAMESPACE(509, Category.NAMESPACE),
+ BEFORE_UPDATE_NAMESPACE_PROPERTIES(510, Category.NAMESPACE),
+ AFTER_UPDATE_NAMESPACE_PROPERTIES(511, Category.NAMESPACE),
// Table Events
- BEFORE_CREATE_TABLE(600),
- AFTER_CREATE_TABLE(601),
- BEFORE_LIST_TABLES(602),
- AFTER_LIST_TABLES(603),
- BEFORE_LOAD_TABLE(604),
- AFTER_LOAD_TABLE(605),
- BEFORE_CHECK_EXISTS_TABLE(606),
- AFTER_CHECK_EXISTS_TABLE(607),
- BEFORE_DROP_TABLE(608),
- AFTER_DROP_TABLE(609),
- BEFORE_REGISTER_TABLE(610),
- AFTER_REGISTER_TABLE(611),
- BEFORE_RENAME_TABLE(612),
- AFTER_RENAME_TABLE(613),
- BEFORE_UPDATE_TABLE(614),
- AFTER_UPDATE_TABLE(615),
- BEFORE_REFRESH_TABLE(616),
- AFTER_REFRESH_TABLE(617),
+ BEFORE_CREATE_TABLE(600, Category.TABLE),
+ AFTER_CREATE_TABLE(601, Category.TABLE),
+ BEFORE_LIST_TABLES(602, Category.TABLE),
+ AFTER_LIST_TABLES(603, Category.TABLE),
+ BEFORE_LOAD_TABLE(604, Category.TABLE),
+ AFTER_LOAD_TABLE(605, Category.TABLE),
+ BEFORE_CHECK_EXISTS_TABLE(606, Category.TABLE),
+ AFTER_CHECK_EXISTS_TABLE(607, Category.TABLE),
+ BEFORE_DROP_TABLE(608, Category.TABLE),
+ AFTER_DROP_TABLE(609, Category.TABLE),
+ BEFORE_REGISTER_TABLE(610, Category.TABLE),
+ AFTER_REGISTER_TABLE(611, Category.TABLE),
+ BEFORE_RENAME_TABLE(612, Category.TABLE),
+ AFTER_RENAME_TABLE(613, Category.TABLE),
+ BEFORE_UPDATE_TABLE(614, Category.TABLE),
+ AFTER_UPDATE_TABLE(615, Category.TABLE),
+ BEFORE_REFRESH_TABLE(616, Category.TABLE),
+ AFTER_REFRESH_TABLE(617, Category.TABLE),
// View Events
- BEFORE_CREATE_VIEW(700),
- AFTER_CREATE_VIEW(701),
- BEFORE_LIST_VIEWS(702),
- AFTER_LIST_VIEWS(703),
- BEFORE_LOAD_VIEW(704),
- AFTER_LOAD_VIEW(705),
- BEFORE_CHECK_EXISTS_VIEW(706),
- AFTER_CHECK_EXISTS_VIEW(707),
- BEFORE_DROP_VIEW(708),
- AFTER_DROP_VIEW(709),
- BEFORE_RENAME_VIEW(710),
- AFTER_RENAME_VIEW(711),
- BEFORE_REPLACE_VIEW(712),
- AFTER_REPLACE_VIEW(713),
- BEFORE_REFRESH_VIEW(714),
- AFTER_REFRESH_VIEW(715),
+ BEFORE_CREATE_VIEW(700, Category.VIEW),
+ AFTER_CREATE_VIEW(701, Category.VIEW),
+ BEFORE_LIST_VIEWS(702, Category.VIEW),
+ AFTER_LIST_VIEWS(703, Category.VIEW),
+ BEFORE_LOAD_VIEW(704, Category.VIEW),
+ AFTER_LOAD_VIEW(705, Category.VIEW),
+ BEFORE_CHECK_EXISTS_VIEW(706, Category.VIEW),
+ AFTER_CHECK_EXISTS_VIEW(707, Category.VIEW),
+ BEFORE_DROP_VIEW(708, Category.VIEW),
+ AFTER_DROP_VIEW(709, Category.VIEW),
+ BEFORE_RENAME_VIEW(710, Category.VIEW),
+ AFTER_RENAME_VIEW(711, Category.VIEW),
+ BEFORE_REPLACE_VIEW(712, Category.VIEW),
+ AFTER_REPLACE_VIEW(713, Category.VIEW),
+ BEFORE_REFRESH_VIEW(714, Category.VIEW),
+ AFTER_REFRESH_VIEW(715, Category.VIEW),
// Credential Events
- BEFORE_LOAD_CREDENTIALS(800),
- AFTER_LOAD_CREDENTIALS(801),
+ BEFORE_LOAD_CREDENTIALS(800, Category.CREDENTIAL),
+ AFTER_LOAD_CREDENTIALS(801, Category.CREDENTIAL),
// Transaction Events
- BEFORE_COMMIT_TRANSACTION(900),
- AFTER_COMMIT_TRANSACTION(901),
+ BEFORE_COMMIT_TRANSACTION(900, Category.TRANSACTION),
+ AFTER_COMMIT_TRANSACTION(901, Category.TRANSACTION),
// Notification Events
- BEFORE_SEND_NOTIFICATION(1000),
- AFTER_SEND_NOTIFICATION(1001),
+ BEFORE_SEND_NOTIFICATION(1000, Category.NOTIFICATION),
+ AFTER_SEND_NOTIFICATION(1001, Category.NOTIFICATION),
// Configuration Events
- BEFORE_GET_CONFIG(1100),
- AFTER_GET_CONFIG(1101),
+ BEFORE_GET_CONFIG(1100, Category.CONFIG),
+ AFTER_GET_CONFIG(1101, Category.CONFIG),
// Policy Events
- BEFORE_CREATE_POLICY(1200),
- AFTER_CREATE_POLICY(1201),
- BEFORE_LIST_POLICIES(1202),
- AFTER_LIST_POLICIES(1203),
- BEFORE_LOAD_POLICY(1204),
- AFTER_LOAD_POLICY(1205),
- BEFORE_UPDATE_POLICY(1206),
- AFTER_UPDATE_POLICY(1207),
- BEFORE_DROP_POLICY(1208),
- AFTER_DROP_POLICY(1209),
- BEFORE_ATTACH_POLICY(1210),
- AFTER_ATTACH_POLICY(1211),
- BEFORE_DETACH_POLICY(1212),
- AFTER_DETACH_POLICY(1213),
- BEFORE_GET_APPLICABLE_POLICIES(1214),
- AFTER_GET_APPLICABLE_POLICIES(1215),
+ BEFORE_CREATE_POLICY(1200, Category.POLICY),
+ AFTER_CREATE_POLICY(1201, Category.POLICY),
+ BEFORE_LIST_POLICIES(1202, Category.POLICY),
+ AFTER_LIST_POLICIES(1203, Category.POLICY),
+ BEFORE_LOAD_POLICY(1204, Category.POLICY),
+ AFTER_LOAD_POLICY(1205, Category.POLICY),
+ BEFORE_UPDATE_POLICY(1206, Category.POLICY),
+ AFTER_UPDATE_POLICY(1207, Category.POLICY),
+ BEFORE_DROP_POLICY(1208, Category.POLICY),
+ AFTER_DROP_POLICY(1209, Category.POLICY),
+ BEFORE_ATTACH_POLICY(1210, Category.POLICY),
+ AFTER_ATTACH_POLICY(1211, Category.POLICY),
+ BEFORE_DETACH_POLICY(1212, Category.POLICY),
+ AFTER_DETACH_POLICY(1213, Category.POLICY),
+ BEFORE_GET_APPLICABLE_POLICIES(1214, Category.POLICY),
+ AFTER_GET_APPLICABLE_POLICIES(1215, Category.POLICY),
// Generic Table Events
- BEFORE_CREATE_GENERIC_TABLE(1300),
- AFTER_CREATE_GENERIC_TABLE(1301),
- BEFORE_DROP_GENERIC_TABLE(1302),
- AFTER_DROP_GENERIC_TABLE(1303),
- BEFORE_LIST_GENERIC_TABLES(1304),
- AFTER_LIST_GENERIC_TABLES(1305),
- BEFORE_LOAD_GENERIC_TABLE(1306),
- AFTER_LOAD_GENERIC_TABLE(1307),
+ BEFORE_CREATE_GENERIC_TABLE(1300, Category.GENERIC_TABLE),
+ AFTER_CREATE_GENERIC_TABLE(1301, Category.GENERIC_TABLE),
+ BEFORE_DROP_GENERIC_TABLE(1302, Category.GENERIC_TABLE),
+ AFTER_DROP_GENERIC_TABLE(1303, Category.GENERIC_TABLE),
+ BEFORE_LIST_GENERIC_TABLES(1304, Category.GENERIC_TABLE),
+ AFTER_LIST_GENERIC_TABLES(1305, Category.GENERIC_TABLE),
+ BEFORE_LOAD_GENERIC_TABLE(1306, Category.GENERIC_TABLE),
+ AFTER_LOAD_GENERIC_TABLE(1307, Category.GENERIC_TABLE),
// Task Execution Events
- BEFORE_ATTEMPT_TASK(1400),
- AFTER_ATTEMPT_TASK(1401),
+ BEFORE_ATTEMPT_TASK(1400, Category.TASK_EXECUTION),
+ AFTER_ATTEMPT_TASK(1401, Category.TASK_EXECUTION),
// Rate Limiting Events
- BEFORE_LIMIT_REQUEST_RATE(1500);
+ BEFORE_LIMIT_REQUEST_RATE(1500, Category.RATE_LIMITING);
private final int code;
+ private final Category category;
- private PolarisEventType(int code) {
+ PolarisEventType(int code, Category category) {
this.code = code;
+ this.category = category;
}
public int code() {
return code;
}
+
+ public Category category() {
+ return category;
+ }
+
+ public static Set<PolarisEventType> typesOfCategory(Category category) {
Review Comment:
That's true, the benefit would be neglibile.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4182114758
> co-author
Sure, that's a great idea!
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3029085905
##########
site/content/in-dev/unreleased/configuration/config-sections/smallrye-polaris_event_listener.md:
##########
@@ -25,5 +25,6 @@ build:
| Property | Default Value | Type | Description |
|----------|---------------|------|-------------|
-| `polaris.event-listener.type` | | `string` | The type of the event listener to use. Must be a registered (`PolarisEventListener`) identifier. <br><br>_Deprecated_ since 1.5.0, use 'polaris.event-listener.types' instead |
+| `polaris.event-listener.type` | | `string` | The type of the event listener to use. Must be a registered (`PolarisEventListener`) identifier. <br><br>_Deprecated_ since 1.5.0, use 'polaris.event-listener.types' instead, if both are set, then polaris.event-listener.types is prioritized |
| `polaris.event-listener.types` | | `list of string` | Comma separated list of event listers, each item must be a registered (`PolarisEventListener`) identifier. |
+| `polaris.event-listener.`_`<name>`_`.enabled-event-types` | | `list of BEFORE_CREATE_CATALOG, AFTER_CREATE_CATALOG, BEFORE_DELETE_CATALOG, AFTER_DELETE_CATALOG, BEFORE_GET_CATALOG, AFTER_GET_CATALOG, BEFORE_UPDATE_CATALOG, AFTER_UPDATE_CATALOG, BEFORE_LIST_CATALOGS, AFTER_LIST_CATALOGS, BEFORE_CREATE_CATALOG_ROLE, AFTER_CREATE_CATALOG_ROLE, BEFORE_DELETE_CATALOG_ROLE, AFTER_DELETE_CATALOG_ROLE, BEFORE_GET_CATALOG_ROLE, AFTER_GET_CATALOG_ROLE, BEFORE_UPDATE_CATALOG_ROLE, AFTER_UPDATE_CATALOG_ROLE, BEFORE_LIST_CATALOG_ROLES, AFTER_LIST_CATALOG_ROLES, BEFORE_ADD_GRANT_TO_CATALOG_ROLE, AFTER_ADD_GRANT_TO_CATALOG_ROLE, BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE, AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE, BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE, AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE, BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE, AFTER_LIST_GRANTS_FOR_CATALOG_ROLE, BEFORE_CREATE_PRINCIPAL, AFTER_CREATE_PRINCIPAL, BEFORE_DELETE_PRINCIPAL, AFTER_DELETE_PRINCIPAL, BEFORE_GET_PRIN
CIPAL, AFTER_GET_PRINCIPAL, BEFORE_UPDATE_PRINCIPAL, AFTER_UPDATE_PRINCIPAL, BEFORE_ROTATE_CREDENTIALS, AFTER_ROTATE_CREDENTIALS, BEFORE_RESET_CREDENTIALS, AFTER_RESET_CREDENTIALS, BEFORE_LIST_PRINCIPALS, AFTER_LIST_PRINCIPALS, BEFORE_ASSIGN_PRINCIPAL_ROLE, AFTER_ASSIGN_PRINCIPAL_ROLE, BEFORE_REVOKE_PRINCIPAL_ROLE, AFTER_REVOKE_PRINCIPAL_ROLE, BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES, AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES, BEFORE_CREATE_PRINCIPAL_ROLE, AFTER_CREATE_PRINCIPAL_ROLE, BEFORE_DELETE_PRINCIPAL_ROLE, AFTER_DELETE_PRINCIPAL_ROLE, BEFORE_GET_PRINCIPAL_ROLE, AFTER_GET_PRINCIPAL_ROLE, BEFORE_UPDATE_PRINCIPAL_ROLE, AFTER_UPDATE_PRINCIPAL_ROLE, BEFORE_LIST_PRINCIPAL_ROLES, AFTER_LIST_PRINCIPAL_ROLES, BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE, AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE, BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE, AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE, BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE, AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE,
BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE, AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE, BEFORE_CREATE_NAMESPACE, AFTER_CREATE_NAMESPACE, BEFORE_LIST_NAMESPACES, AFTER_LIST_NAMESPACES, BEFORE_LOAD_NAMESPACE_METADATA, AFTER_LOAD_NAMESPACE_METADATA, BEFORE_CHECK_EXISTS_NAMESPACE, AFTER_CHECK_EXISTS_NAMESPACE, BEFORE_DROP_NAMESPACE, AFTER_DROP_NAMESPACE, BEFORE_UPDATE_NAMESPACE_PROPERTIES, AFTER_UPDATE_NAMESPACE_PROPERTIES, BEFORE_CREATE_TABLE, AFTER_CREATE_TABLE, BEFORE_LIST_TABLES, AFTER_LIST_TABLES, BEFORE_LOAD_TABLE, AFTER_LOAD_TABLE, BEFORE_CHECK_EXISTS_TABLE, AFTER_CHECK_EXISTS_TABLE, BEFORE_DROP_TABLE, AFTER_DROP_TABLE, BEFORE_REGISTER_TABLE, AFTER_REGISTER_TABLE, BEFORE_RENAME_TABLE, AFTER_RENAME_TABLE, BEFORE_UPDATE_TABLE, AFTER_UPDATE_TABLE, BEFORE_REFRESH_TABLE, AFTER_REFRESH_TABLE, BEFORE_CREATE_VIEW, AFTER_CREATE_VIEW, BEFORE_LIST_VIEWS, AFTER_LIST_VIEWS, BEFORE_LOAD_VIEW, AFTER_LOAD_VIEW, BEFORE_CHECK_EXISTS_VIEW, AFTER_CHECK_EXISTS_VIEW, BEFORE_DROP_VIEW, AFTER_DROP_VI
EW, BEFORE_RENAME_VIEW, AFTER_RENAME_VIEW, BEFORE_REPLACE_VIEW, AFTER_REPLACE_VIEW, BEFORE_REFRESH_VIEW, AFTER_REFRESH_VIEW, BEFORE_LOAD_CREDENTIALS, AFTER_LOAD_CREDENTIALS, BEFORE_COMMIT_TRANSACTION, AFTER_COMMIT_TRANSACTION, BEFORE_SEND_NOTIFICATION, AFTER_SEND_NOTIFICATION, BEFORE_GET_CONFIG, AFTER_GET_CONFIG, BEFORE_CREATE_POLICY, AFTER_CREATE_POLICY, BEFORE_LIST_POLICIES, AFTER_LIST_POLICIES, BEFORE_LOAD_POLICY, AFTER_LOAD_POLICY, BEFORE_UPDATE_POLICY, AFTER_UPDATE_POLICY, BEFORE_DROP_POLICY, AFTER_DROP_POLICY, BEFORE_ATTACH_POLICY, AFTER_ATTACH_POLICY, BEFORE_DETACH_POLICY, AFTER_DETACH_POLICY, BEFORE_GET_APPLICABLE_POLICIES, AFTER_GET_APPLICABLE_POLICIES, BEFORE_CREATE_GENERIC_TABLE, AFTER_CREATE_GENERIC_TABLE, BEFORE_DROP_GENERIC_TABLE, AFTER_DROP_GENERIC_TABLE, BEFORE_LIST_GENERIC_TABLES, AFTER_LIST_GENERIC_TABLES, BEFORE_LOAD_GENERIC_TABLE, AFTER_LOAD_GENERIC_TABLE, BEFORE_ATTEMPT_TASK, AFTER_ATTEMPT_TASK, BEFORE_LIMIT_REQUEST_RATE` | Comma separated list of enabled event
types. This event listener will only receive events of the selected types. |
Review Comment:
To avoid 150 different event types, we could consider each event to have a 'category' or 'entityType'. One can then configure an event listener to consume only a set of event categories, instead of listing all of the event which are related to for example Catalog.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3029185958
##########
runtime/service/src/main/java/org/apache/polaris/service/events/jsonEventListener/aws/cloudwatch/AwsCloudWatchEventListener.java:
##########
@@ -146,9 +145,6 @@ void shutdown() {
@Override
public void onEvent(PolarisEvent event) {
- if (event.type() != PolarisEventType.AFTER_REFRESH_TABLE) {
Review Comment:
The feature is flagged as beta/experimental, so I wouldn't really care about that β also, it's a weird default value imho.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4245349795
Good point about CHANGELOG. This PR affects how user configure Polaris), but I believe this change is fully backward compatible, right?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4064:
URL: https://github.com/apache/polaris/pull/4064#issuecomment-4245388895
> Good point about CHANGELOG. This PR affects how users configure Polaris), but I believe this change is fully backward compatible, right?
Yes, it should be. Or well, not sure if we should consider this: https://github.com/apache/polaris/pull/4064#discussion_r3029096621 as an incompatible modification or not.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3052514400
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventListenersTest.java:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.events;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import com.google.common.collect.ImmutableMap;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.QuarkusTestProfile;
+import io.quarkus.test.junit.TestProfile;
+import io.smallrye.common.annotation.Identifier;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(PolarisEventListenersTest.PolarisEventListenersTestProfile.class)
+public class PolarisEventListenersTest {
+ static final Set<PolarisEventType> CATALOG_EVENTS =
+ Arrays.stream(PolarisEventType.values())
+ .filter(e -> e.category() == PolarisEventType.Category.CATALOG)
+ .collect(Collectors.toSet());
+
+ static final CountDownLatch latch = new CountDownLatch(5 + 3 * CATALOG_EVENTS.size());
Review Comment:
Awesome, I can try it out in this test too!
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3067170635
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisServiceBusEventDispatcher.java:
##########
@@ -40,6 +41,11 @@ public class PolarisServiceBusEventDispatcher implements PolarisEventDispatcher
*/
@Override
public void dispatch(PolarisEvent event) {
- eventBus.publish(POLARIS_EVENT_CHANNEL, event);
+ eventBus.publish(POLARIS_EVENT_CHANNEL + "." + event.type(), event);
Review Comment:
nit: would it be reasonable to double check that the event type is enabled? I suppose the overhead is not huge, but the benefit is safeguarding against mistakes in enabled checks at call sites π€
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3071540114
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -31,23 +31,68 @@
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;
+import java.util.BitSet;
+import java.util.EnumSet;
import java.util.HashSet;
-import java.util.Set;
import org.apache.polaris.service.events.listeners.PolarisEventListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@ApplicationScoped
public class PolarisEventListeners {
+ private static final Logger LOGGER = LoggerFactory.getLogger(PolarisEventListeners.class);
+
@Inject EventBus eventBus;
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
Review Comment:
Makes sense to me, as EnumSet is implemented as a bit vector, the complexity of a lookup should be the same for both. @adutra what do you think?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r3074565805
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java:
##########
@@ -19,189 +19,223 @@
package org.apache.polaris.service.events;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
public enum PolarisEventType {
// Catalog Events
- BEFORE_CREATE_CATALOG(100),
- AFTER_CREATE_CATALOG(101),
- BEFORE_DELETE_CATALOG(102),
- AFTER_DELETE_CATALOG(103),
- BEFORE_GET_CATALOG(104),
- AFTER_GET_CATALOG(105),
- BEFORE_UPDATE_CATALOG(106),
- AFTER_UPDATE_CATALOG(107),
- BEFORE_LIST_CATALOGS(108),
- AFTER_LIST_CATALOGS(109),
+ BEFORE_CREATE_CATALOG(100, Category.CATALOG),
+ AFTER_CREATE_CATALOG(101, Category.CATALOG),
+ BEFORE_DELETE_CATALOG(102, Category.CATALOG),
+ AFTER_DELETE_CATALOG(103, Category.CATALOG),
+ BEFORE_GET_CATALOG(104, Category.CATALOG),
+ AFTER_GET_CATALOG(105, Category.CATALOG),
+ BEFORE_UPDATE_CATALOG(106, Category.CATALOG),
+ AFTER_UPDATE_CATALOG(107, Category.CATALOG),
+ BEFORE_LIST_CATALOGS(108, Category.CATALOG),
+ AFTER_LIST_CATALOGS(109, Category.CATALOG),
// Catalog Role Events
- BEFORE_CREATE_CATALOG_ROLE(200),
- AFTER_CREATE_CATALOG_ROLE(201),
- BEFORE_DELETE_CATALOG_ROLE(202),
- AFTER_DELETE_CATALOG_ROLE(203),
- BEFORE_GET_CATALOG_ROLE(204),
- AFTER_GET_CATALOG_ROLE(205),
- BEFORE_UPDATE_CATALOG_ROLE(206),
- AFTER_UPDATE_CATALOG_ROLE(207),
- BEFORE_LIST_CATALOG_ROLES(208),
- AFTER_LIST_CATALOG_ROLES(209),
- BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210),
- AFTER_ADD_GRANT_TO_CATALOG_ROLE(211),
- BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212),
- AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213),
- BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214),
- AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215),
- BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216),
- AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217),
+ BEFORE_CREATE_CATALOG_ROLE(200, Category.CATALOG_ROLE),
+ AFTER_CREATE_CATALOG_ROLE(201, Category.CATALOG_ROLE),
+ BEFORE_DELETE_CATALOG_ROLE(202, Category.CATALOG_ROLE),
+ AFTER_DELETE_CATALOG_ROLE(203, Category.CATALOG_ROLE),
+ BEFORE_GET_CATALOG_ROLE(204, Category.CATALOG_ROLE),
+ AFTER_GET_CATALOG_ROLE(205, Category.CATALOG_ROLE),
+ BEFORE_UPDATE_CATALOG_ROLE(206, Category.CATALOG_ROLE),
+ AFTER_UPDATE_CATALOG_ROLE(207, Category.CATALOG_ROLE),
+ BEFORE_LIST_CATALOG_ROLES(208, Category.CATALOG_ROLE),
+ AFTER_LIST_CATALOG_ROLES(209, Category.CATALOG_ROLE),
+ BEFORE_ADD_GRANT_TO_CATALOG_ROLE(210, Category.CATALOG_ROLE),
+ AFTER_ADD_GRANT_TO_CATALOG_ROLE(211, Category.CATALOG_ROLE),
+ BEFORE_REVOKE_GRANT_FROM_CATALOG_ROLE(212, Category.CATALOG_ROLE),
+ AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE(213, Category.CATALOG_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(214, Category.CATALOG_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPAL_ROLES_FOR_CATALOG_ROLE(215, Category.CATALOG_ROLE),
+ BEFORE_LIST_GRANTS_FOR_CATALOG_ROLE(216, Category.CATALOG_ROLE),
+ AFTER_LIST_GRANTS_FOR_CATALOG_ROLE(217, Category.CATALOG_ROLE),
// Principal Events
- BEFORE_CREATE_PRINCIPAL(300),
- AFTER_CREATE_PRINCIPAL(301),
- BEFORE_DELETE_PRINCIPAL(302),
- AFTER_DELETE_PRINCIPAL(303),
- BEFORE_GET_PRINCIPAL(304),
- AFTER_GET_PRINCIPAL(305),
- BEFORE_UPDATE_PRINCIPAL(306),
- AFTER_UPDATE_PRINCIPAL(307),
- BEFORE_ROTATE_CREDENTIALS(308),
- AFTER_ROTATE_CREDENTIALS(309),
- BEFORE_RESET_CREDENTIALS(310),
- AFTER_RESET_CREDENTIALS(311),
- BEFORE_LIST_PRINCIPALS(312),
- AFTER_LIST_PRINCIPALS(313),
- BEFORE_ASSIGN_PRINCIPAL_ROLE(314),
- AFTER_ASSIGN_PRINCIPAL_ROLE(315),
- BEFORE_REVOKE_PRINCIPAL_ROLE(316),
- AFTER_REVOKE_PRINCIPAL_ROLE(317),
- BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318),
- AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319),
+ BEFORE_CREATE_PRINCIPAL(300, Category.PRINCIPAL),
+ AFTER_CREATE_PRINCIPAL(301, Category.PRINCIPAL),
+ BEFORE_DELETE_PRINCIPAL(302, Category.PRINCIPAL),
+ AFTER_DELETE_PRINCIPAL(303, Category.PRINCIPAL),
+ BEFORE_GET_PRINCIPAL(304, Category.PRINCIPAL),
+ AFTER_GET_PRINCIPAL(305, Category.PRINCIPAL),
+ BEFORE_UPDATE_PRINCIPAL(306, Category.PRINCIPAL),
+ AFTER_UPDATE_PRINCIPAL(307, Category.PRINCIPAL),
+ BEFORE_ROTATE_CREDENTIALS(308, Category.PRINCIPAL),
+ AFTER_ROTATE_CREDENTIALS(309, Category.PRINCIPAL),
+ BEFORE_RESET_CREDENTIALS(310, Category.PRINCIPAL),
+ AFTER_RESET_CREDENTIALS(311, Category.PRINCIPAL),
+ BEFORE_LIST_PRINCIPALS(312, Category.PRINCIPAL),
+ AFTER_LIST_PRINCIPALS(313, Category.PRINCIPAL),
+ BEFORE_ASSIGN_PRINCIPAL_ROLE(314, Category.PRINCIPAL),
+ AFTER_ASSIGN_PRINCIPAL_ROLE(315, Category.PRINCIPAL),
+ BEFORE_REVOKE_PRINCIPAL_ROLE(316, Category.PRINCIPAL),
+ AFTER_REVOKE_PRINCIPAL_ROLE(317, Category.PRINCIPAL),
+ BEFORE_LIST_ASSIGNED_PRINCIPAL_ROLES(318, Category.PRINCIPAL),
+ AFTER_LIST_ASSIGNED_PRINCIPAL_ROLES(319, Category.PRINCIPAL),
// Principal Role Events
- BEFORE_CREATE_PRINCIPAL_ROLE(400),
- AFTER_CREATE_PRINCIPAL_ROLE(401),
- BEFORE_DELETE_PRINCIPAL_ROLE(402),
- AFTER_DELETE_PRINCIPAL_ROLE(403),
- BEFORE_GET_PRINCIPAL_ROLE(404),
- AFTER_GET_PRINCIPAL_ROLE(405),
- BEFORE_UPDATE_PRINCIPAL_ROLE(406),
- AFTER_UPDATE_PRINCIPAL_ROLE(407),
- BEFORE_LIST_PRINCIPAL_ROLES(408),
- AFTER_LIST_PRINCIPAL_ROLES(409),
- BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410),
- AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411),
- BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412),
- AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413),
- BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414),
- AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415),
- BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416),
- AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417),
+ BEFORE_CREATE_PRINCIPAL_ROLE(400, Category.PRINCIPAL_ROLE),
+ AFTER_CREATE_PRINCIPAL_ROLE(401, Category.PRINCIPAL_ROLE),
+ BEFORE_DELETE_PRINCIPAL_ROLE(402, Category.PRINCIPAL_ROLE),
+ AFTER_DELETE_PRINCIPAL_ROLE(403, Category.PRINCIPAL_ROLE),
+ BEFORE_GET_PRINCIPAL_ROLE(404, Category.PRINCIPAL_ROLE),
+ AFTER_GET_PRINCIPAL_ROLE(405, Category.PRINCIPAL_ROLE),
+ BEFORE_UPDATE_PRINCIPAL_ROLE(406, Category.PRINCIPAL_ROLE),
+ AFTER_UPDATE_PRINCIPAL_ROLE(407, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_PRINCIPAL_ROLES(408, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_PRINCIPAL_ROLES(409, Category.PRINCIPAL_ROLE),
+ BEFORE_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(410, Category.PRINCIPAL_ROLE),
+ AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(411, Category.PRINCIPAL_ROLE),
+ BEFORE_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(412, Category.PRINCIPAL_ROLE),
+ AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(413, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(414, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(415, Category.PRINCIPAL_ROLE),
+ BEFORE_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(416, Category.PRINCIPAL_ROLE),
+ AFTER_LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(417, Category.PRINCIPAL_ROLE),
// Namespace Events
- BEFORE_CREATE_NAMESPACE(500),
- AFTER_CREATE_NAMESPACE(501),
- BEFORE_LIST_NAMESPACES(502),
- AFTER_LIST_NAMESPACES(503),
- BEFORE_LOAD_NAMESPACE_METADATA(504),
- AFTER_LOAD_NAMESPACE_METADATA(505),
- BEFORE_CHECK_EXISTS_NAMESPACE(506),
- AFTER_CHECK_EXISTS_NAMESPACE(507),
- BEFORE_DROP_NAMESPACE(508),
- AFTER_DROP_NAMESPACE(509),
- BEFORE_UPDATE_NAMESPACE_PROPERTIES(510),
- AFTER_UPDATE_NAMESPACE_PROPERTIES(511),
+ BEFORE_CREATE_NAMESPACE(500, Category.NAMESPACE),
+ AFTER_CREATE_NAMESPACE(501, Category.NAMESPACE),
+ BEFORE_LIST_NAMESPACES(502, Category.NAMESPACE),
+ AFTER_LIST_NAMESPACES(503, Category.NAMESPACE),
+ BEFORE_LOAD_NAMESPACE_METADATA(504, Category.NAMESPACE),
+ AFTER_LOAD_NAMESPACE_METADATA(505, Category.NAMESPACE),
+ BEFORE_CHECK_EXISTS_NAMESPACE(506, Category.NAMESPACE),
+ AFTER_CHECK_EXISTS_NAMESPACE(507, Category.NAMESPACE),
+ BEFORE_DROP_NAMESPACE(508, Category.NAMESPACE),
+ AFTER_DROP_NAMESPACE(509, Category.NAMESPACE),
+ BEFORE_UPDATE_NAMESPACE_PROPERTIES(510, Category.NAMESPACE),
+ AFTER_UPDATE_NAMESPACE_PROPERTIES(511, Category.NAMESPACE),
// Table Events
- BEFORE_CREATE_TABLE(600),
- AFTER_CREATE_TABLE(601),
- BEFORE_LIST_TABLES(602),
- AFTER_LIST_TABLES(603),
- BEFORE_LOAD_TABLE(604),
- AFTER_LOAD_TABLE(605),
- BEFORE_CHECK_EXISTS_TABLE(606),
- AFTER_CHECK_EXISTS_TABLE(607),
- BEFORE_DROP_TABLE(608),
- AFTER_DROP_TABLE(609),
- BEFORE_REGISTER_TABLE(610),
- AFTER_REGISTER_TABLE(611),
- BEFORE_RENAME_TABLE(612),
- AFTER_RENAME_TABLE(613),
- BEFORE_UPDATE_TABLE(614),
- AFTER_UPDATE_TABLE(615),
- BEFORE_REFRESH_TABLE(616),
- AFTER_REFRESH_TABLE(617),
+ BEFORE_CREATE_TABLE(600, Category.TABLE),
+ AFTER_CREATE_TABLE(601, Category.TABLE),
+ BEFORE_LIST_TABLES(602, Category.TABLE),
+ AFTER_LIST_TABLES(603, Category.TABLE),
+ BEFORE_LOAD_TABLE(604, Category.TABLE),
+ AFTER_LOAD_TABLE(605, Category.TABLE),
+ BEFORE_CHECK_EXISTS_TABLE(606, Category.TABLE),
+ AFTER_CHECK_EXISTS_TABLE(607, Category.TABLE),
+ BEFORE_DROP_TABLE(608, Category.TABLE),
+ AFTER_DROP_TABLE(609, Category.TABLE),
+ BEFORE_REGISTER_TABLE(610, Category.TABLE),
+ AFTER_REGISTER_TABLE(611, Category.TABLE),
+ BEFORE_RENAME_TABLE(612, Category.TABLE),
+ AFTER_RENAME_TABLE(613, Category.TABLE),
+ BEFORE_UPDATE_TABLE(614, Category.TABLE),
+ AFTER_UPDATE_TABLE(615, Category.TABLE),
+ BEFORE_REFRESH_TABLE(616, Category.TABLE),
+ AFTER_REFRESH_TABLE(617, Category.TABLE),
// View Events
- BEFORE_CREATE_VIEW(700),
- AFTER_CREATE_VIEW(701),
- BEFORE_LIST_VIEWS(702),
- AFTER_LIST_VIEWS(703),
- BEFORE_LOAD_VIEW(704),
- AFTER_LOAD_VIEW(705),
- BEFORE_CHECK_EXISTS_VIEW(706),
- AFTER_CHECK_EXISTS_VIEW(707),
- BEFORE_DROP_VIEW(708),
- AFTER_DROP_VIEW(709),
- BEFORE_RENAME_VIEW(710),
- AFTER_RENAME_VIEW(711),
- BEFORE_REPLACE_VIEW(712),
- AFTER_REPLACE_VIEW(713),
- BEFORE_REFRESH_VIEW(714),
- AFTER_REFRESH_VIEW(715),
+ BEFORE_CREATE_VIEW(700, Category.VIEW),
+ AFTER_CREATE_VIEW(701, Category.VIEW),
+ BEFORE_LIST_VIEWS(702, Category.VIEW),
+ AFTER_LIST_VIEWS(703, Category.VIEW),
+ BEFORE_LOAD_VIEW(704, Category.VIEW),
+ AFTER_LOAD_VIEW(705, Category.VIEW),
+ BEFORE_CHECK_EXISTS_VIEW(706, Category.VIEW),
+ AFTER_CHECK_EXISTS_VIEW(707, Category.VIEW),
+ BEFORE_DROP_VIEW(708, Category.VIEW),
+ AFTER_DROP_VIEW(709, Category.VIEW),
+ BEFORE_RENAME_VIEW(710, Category.VIEW),
+ AFTER_RENAME_VIEW(711, Category.VIEW),
+ BEFORE_REPLACE_VIEW(712, Category.VIEW),
+ AFTER_REPLACE_VIEW(713, Category.VIEW),
+ BEFORE_REFRESH_VIEW(714, Category.VIEW),
+ AFTER_REFRESH_VIEW(715, Category.VIEW),
// Credential Events
- BEFORE_LOAD_CREDENTIALS(800),
- AFTER_LOAD_CREDENTIALS(801),
+ BEFORE_LOAD_CREDENTIALS(800, Category.CREDENTIAL),
+ AFTER_LOAD_CREDENTIALS(801, Category.CREDENTIAL),
// Transaction Events
- BEFORE_COMMIT_TRANSACTION(900),
- AFTER_COMMIT_TRANSACTION(901),
+ BEFORE_COMMIT_TRANSACTION(900, Category.TRANSACTION),
+ AFTER_COMMIT_TRANSACTION(901, Category.TRANSACTION),
// Notification Events
- BEFORE_SEND_NOTIFICATION(1000),
- AFTER_SEND_NOTIFICATION(1001),
+ BEFORE_SEND_NOTIFICATION(1000, Category.NOTIFICATION),
+ AFTER_SEND_NOTIFICATION(1001, Category.NOTIFICATION),
// Configuration Events
- BEFORE_GET_CONFIG(1100),
- AFTER_GET_CONFIG(1101),
+ BEFORE_GET_CONFIG(1100, Category.CONFIG),
+ AFTER_GET_CONFIG(1101, Category.CONFIG),
// Policy Events
- BEFORE_CREATE_POLICY(1200),
- AFTER_CREATE_POLICY(1201),
- BEFORE_LIST_POLICIES(1202),
- AFTER_LIST_POLICIES(1203),
- BEFORE_LOAD_POLICY(1204),
- AFTER_LOAD_POLICY(1205),
- BEFORE_UPDATE_POLICY(1206),
- AFTER_UPDATE_POLICY(1207),
- BEFORE_DROP_POLICY(1208),
- AFTER_DROP_POLICY(1209),
- BEFORE_ATTACH_POLICY(1210),
- AFTER_ATTACH_POLICY(1211),
- BEFORE_DETACH_POLICY(1212),
- AFTER_DETACH_POLICY(1213),
- BEFORE_GET_APPLICABLE_POLICIES(1214),
- AFTER_GET_APPLICABLE_POLICIES(1215),
+ BEFORE_CREATE_POLICY(1200, Category.POLICY),
+ AFTER_CREATE_POLICY(1201, Category.POLICY),
+ BEFORE_LIST_POLICIES(1202, Category.POLICY),
+ AFTER_LIST_POLICIES(1203, Category.POLICY),
+ BEFORE_LOAD_POLICY(1204, Category.POLICY),
+ AFTER_LOAD_POLICY(1205, Category.POLICY),
+ BEFORE_UPDATE_POLICY(1206, Category.POLICY),
+ AFTER_UPDATE_POLICY(1207, Category.POLICY),
+ BEFORE_DROP_POLICY(1208, Category.POLICY),
+ AFTER_DROP_POLICY(1209, Category.POLICY),
+ BEFORE_ATTACH_POLICY(1210, Category.POLICY),
+ AFTER_ATTACH_POLICY(1211, Category.POLICY),
+ BEFORE_DETACH_POLICY(1212, Category.POLICY),
+ AFTER_DETACH_POLICY(1213, Category.POLICY),
+ BEFORE_GET_APPLICABLE_POLICIES(1214, Category.POLICY),
+ AFTER_GET_APPLICABLE_POLICIES(1215, Category.POLICY),
// Generic Table Events
- BEFORE_CREATE_GENERIC_TABLE(1300),
- AFTER_CREATE_GENERIC_TABLE(1301),
- BEFORE_DROP_GENERIC_TABLE(1302),
- AFTER_DROP_GENERIC_TABLE(1303),
- BEFORE_LIST_GENERIC_TABLES(1304),
- AFTER_LIST_GENERIC_TABLES(1305),
- BEFORE_LOAD_GENERIC_TABLE(1306),
- AFTER_LOAD_GENERIC_TABLE(1307),
+ BEFORE_CREATE_GENERIC_TABLE(1300, Category.GENERIC_TABLE),
+ AFTER_CREATE_GENERIC_TABLE(1301, Category.GENERIC_TABLE),
+ BEFORE_DROP_GENERIC_TABLE(1302, Category.GENERIC_TABLE),
+ AFTER_DROP_GENERIC_TABLE(1303, Category.GENERIC_TABLE),
+ BEFORE_LIST_GENERIC_TABLES(1304, Category.GENERIC_TABLE),
+ AFTER_LIST_GENERIC_TABLES(1305, Category.GENERIC_TABLE),
+ BEFORE_LOAD_GENERIC_TABLE(1306, Category.GENERIC_TABLE),
+ AFTER_LOAD_GENERIC_TABLE(1307, Category.GENERIC_TABLE),
// Task Execution Events
- BEFORE_ATTEMPT_TASK(1400),
- AFTER_ATTEMPT_TASK(1401),
+ BEFORE_ATTEMPT_TASK(1400, Category.TASK_EXECUTION),
+ AFTER_ATTEMPT_TASK(1401, Category.TASK_EXECUTION),
// Rate Limiting Events
- BEFORE_LIMIT_REQUEST_RATE(1500);
+ BEFORE_LIMIT_REQUEST_RATE(1500, Category.RATE_LIMITING);
private final int code;
+ private final Category category;
- private PolarisEventType(int code) {
+ PolarisEventType(int code, Category category) {
this.code = code;
+ this.category = category;
}
public int code() {
return code;
}
+
+ public Category category() {
+ return category;
+ }
+
+ public static Set<PolarisEventType> typesOfCategory(Category category) {
Review Comment:
the overhead of filtering ~200 enum values on startup is negligible, even if done a few times, IMHO.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create PolarisEvent only when event type is enabled by at least one event listener [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4064:
URL: https://github.com/apache/polaris/pull/4064#discussion_r2995705191
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisServiceBusEventDispatcher.java:
##########
@@ -40,6 +41,11 @@ public class PolarisServiceBusEventDispatcher implements PolarisEventDispatcher
*/
@Override
public void dispatch(PolarisEvent event) {
- eventBus.publish(POLARIS_EVENT_CHANNEL, event);
+ eventBus.publish(POLARIS_EVENT_CHANNEL + "." + event, event);
Review Comment:
Same here. There should be one channel per *event type*.
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListeners.java:
##########
@@ -41,13 +41,26 @@ public class PolarisEventListeners {
@Inject @Any Instance<PolarisEventListener> eventListeners;
@Inject PolarisEventListenerConfiguration configuration;
+ private final BitSet eventsByType = new BitSet(PolarisEventType.values().length);
+
public void onStartup(@Observes StartupEvent event) {
- Set<String> listenerTypeSet = configuration.types().orElseGet(HashSet::new);
+ var listenerTypeSet = configuration.types().orElseGet(HashSet::new);
for (String enabledEventListener : listenerTypeSet) {
- PolarisEventListener listener =
- eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
+ var listenerConfiguration = configuration.listenerConfig().get(enabledEventListener);
+ var supportedTypes = PolarisEventType.values();
+ if (listenerConfiguration != null && !listenerConfiguration.enabledEventTypes().isEmpty()) {
+ supportedTypes = listenerConfiguration.enabledEventTypes().toArray(PolarisEventType[]::new);
+ }
+ for (var polarisEventType : supportedTypes) {
+ eventsByType.set(polarisEventType.ordinal());
+ }
+ var listener = eventListeners.select(Identifier.Literal.of(enabledEventListener)).get();
Handler<Message<PolarisEvent>> handler = e -> listener.onEvent(e.body());
- eventBus.localConsumer(POLARIS_EVENT_CHANNEL, handler);
+ eventBus.localConsumer(POLARIS_EVENT_CHANNEL + "." + event, handler);
Review Comment:
This doesn't look correct to me. Appending the startup event to the channel name does not make sense imho.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org