adnanhemani commented on code in PR #2480:
URL: https://github.com/apache/polaris/pull/2480#discussion_r2320481928
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java:
##########
@@ -777,8 +777,8 @@ public Response sendNotification(
securityContext,
prefix,
catalog -> {
- catalog.sendNotification(tableIdentifier, notificationRequest);
- return Response.status(Response.Status.NO_CONTENT).build();
+ boolean notificationSent = catalog.sendNotification(tableIdentifier,
notificationRequest);
Review Comment:
Needing an OpenAPI spec change and ML thread regarding this is a fair ask.
Let me revert this out for now - maybe we can proceed without the result of the
`sendNotification` call first and then in parallel, I can start the ML thread
for this.
Once we reach a consensus as a community, then we can go back and resolve
the TODO.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -307,7 +519,18 @@ public Response sendNotification(
NotificationRequest notificationRequest,
RealmContext realmContext,
SecurityContext securityContext) {
- return delegate.sendNotification(
- prefix, namespace, table, notificationRequest, realmContext,
securityContext);
+ String catalogName = getCatalogFromPrefix(prefix, realmContext);
+ polarisEventListener.onBeforeSendNotification(
+ new BeforeSendNotificationEvent(catalogName, namespace, table,
notificationRequest));
+ Response resp =
+ delegate.sendNotification(
+ prefix, namespace, table, notificationRequest, realmContext,
securityContext);
+ polarisEventListener.onAfterSendNotification(
+ new AfterSendNotificationEvent(catalogName, namespace, table,
(boolean) resp.getEntity()));
+ return resp;
+ }
+
+ private String getCatalogFromPrefix(String prefix, RealmContext
realmContext) {
+ return prefixParser.prefixToCatalogName(realmContext, prefix);
Review Comment:
Changed.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -103,8 +192,23 @@ public Response createTable(
String accessDelegationMode,
RealmContext realmContext,
SecurityContext securityContext) {
Review Comment:
Done as part of merge from main.
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventListener.java:
##########
@@ -55,4 +55,162 @@ public void onBeforeTaskAttempted(BeforeTaskAttemptedEvent
event) {}
/** {@link AfterTaskAttemptedEvent} */
public void onAfterTaskAttempted(AfterTaskAttemptedEvent event) {}
+
+ // Iceberg REST Catalog Namespace Events
+ /** {@link IcebergRestCatalogEvents.BeforeCreateNamespaceEvent} */
+ public void
onBeforeCreateNamespace(IcebergRestCatalogEvents.BeforeCreateNamespaceEvent
event) {}
+
+ /** {@link IcebergRestCatalogEvents.AfterCreateNamespaceEvent} */
+ public void
onAfterCreateNamespace(IcebergRestCatalogEvents.AfterCreateNamespaceEvent
event) {}
+
+ /** {@link IcebergRestCatalogEvents.BeforeListNamespacesEvent} */
+ public void
onBeforeListNamespaces(IcebergRestCatalogEvents.BeforeListNamespacesEvent
event) {}
+
+ /** {@link IcebergRestCatalogEvents.AfterListNamespacesEvent} */
+ public void
onAfterListNamespaces(IcebergRestCatalogEvents.AfterListNamespacesEvent event)
{}
+
+ /** {@link IcebergRestCatalogEvents.BeforeLoadNamespaceMetadataEvent} */
+ public void onBeforeLoadNamespaceMetadata(
+ IcebergRestCatalogEvents.BeforeLoadNamespaceMetadataEvent event) {}
+
+ /** {@link IcebergRestCatalogEvents.AfterLoadNamespaceMetadataEvent} */
+ public void onAfterLoadNamespaceMetadata(
+ IcebergRestCatalogEvents.AfterLoadNamespaceMetadataEvent event) {}
+
+ /** {@link IcebergRestCatalogEvents.BeforeCheckNamespaceExistsEvent} */
+ public void onBeforeCheckNamespaceExists(
+ IcebergRestCatalogEvents.BeforeCheckNamespaceExistsEvent event) {}
+
+ /** {@link IcebergRestCatalogEvents.AfterCheckNamespaceExistsEvent} */
+ public void onAfterCheckNamespaceExists(
+ IcebergRestCatalogEvents.AfterCheckNamespaceExistsEvent event) {}
+
+ /** {@link IcebergRestCatalogEvents.BeforeDropNamespaceEvent} */
+ public void
onBeforeDropNamespace(IcebergRestCatalogEvents.BeforeDropNamespaceEvent event)
{}
+
+ /** {@link IcebergRestCatalogEvents.AfterDropNamespaceEvent} */
+ public void
onAfterDropNamespace(IcebergRestCatalogEvents.AfterDropNamespaceEvent event) {}
+
+ /** {@link IcebergRestCatalogEvents.BeforeUpdateNamespacePropertiesEvent} */
+ public void onBeforeUpdateNamespaceProperties(
+ IcebergRestCatalogEvents.BeforeUpdateNamespacePropertiesEvent event) {}
+
+ /** {@link IcebergRestCatalogEvents.AfterUpdateNamespacePropertiesEvent} */
+ public void onAfterUpdateNamespaceProperties(
+ IcebergRestCatalogEvents.AfterUpdateNamespacePropertiesEvent event) {}
+
+ // Iceberg REST Catalog Table Events
Review Comment:
Changed the remaining to be verb-object as well. Pleas let me know if
something still stands out :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
