adnanhemani commented on code in PR #2480:
URL: https://github.com/apache/polaris/pull/2480#discussion_r2317632595
##########
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:
No, at the moment we do not get whether the notification was sent
successfully or not. So I've introduced this change so that we can surface that
information. (There are code paths possible where the notificationSent can be
false but the call still returns without error).
But to be fair, I'm not sure what notificationSent really helps with. So
please let me know what you think.
##########
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:
Good point, changed.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -62,26 +124,45 @@ public Response listNamespaces(
String parent,
RealmContext realmContext,
SecurityContext securityContext) {
- return delegate.listNamespaces(
- prefix, pageToken, pageSize, parent, realmContext, securityContext);
+ polarisEventListener.onBeforeListNamespaces(new
BeforeListNamespacesEvent(prefix, parent));
+ Response resp =
+ delegate.listNamespaces(prefix, pageToken, pageSize, parent,
realmContext, securityContext);
+ polarisEventListener.onAfterListNamespaces(new
AfterListNamespacesEvent(prefix, parent));
Review Comment:
That is the plan for the future, yes. Well, use try-finally rather than
catch.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -44,14 +96,24 @@
public class IcebergRestCatalogEventServiceDelegator implements
IcebergRestCatalogApiService {
@Inject @Delegate IcebergCatalogAdapter delegate;
+ @Inject PolarisEventListener polarisEventListener;
@Override
public Response createNamespace(
String prefix,
CreateNamespaceRequest createNamespaceRequest,
RealmContext realmContext,
SecurityContext securityContext) {
- return delegate.createNamespace(prefix, createNamespaceRequest,
realmContext, securityContext);
+ polarisEventListener.onBeforeCreateNamespace(
+ new BeforeCreateNamespaceEvent(prefix, createNamespaceRequest));
+ Response resp =
+ delegate.createNamespace(prefix, createNamespaceRequest, realmContext,
securityContext);
+ CreateNamespaceResponse createNamespaceResponse =
+ resp.readEntity(CreateNamespaceResponse.class);
Review Comment:
Thanks for pointing this out! Let me change this in the next revision.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestConfigurationEventServiceDelegator.java:
##########
@@ -25,19 +25,28 @@
import jakarta.inject.Inject;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.SecurityContext;
+import org.apache.iceberg.rest.responses.ConfigResponse;
import org.apache.polaris.core.context.RealmContext;
import
org.apache.polaris.service.catalog.api.IcebergRestConfigurationApiService;
+import org.apache.polaris.service.events.IcebergRestCatalogEvents;
+import org.apache.polaris.service.events.PolarisEventListener;
@Decorator
@Priority(1000)
public class IcebergRestConfigurationEventServiceDelegator
implements IcebergRestConfigurationApiService {
Review Comment:
I purposefully left it alone as the `getToken` endpoint is deprecated for
removal. Please let me know if you'd still like me to implement it regardless.
##########
runtime/service/src/main/java/org/apache/polaris/service/events/IcebergRestCatalogEvents.java:
##########
@@ -0,0 +1,179 @@
+/*
+ * 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 java.util.Map;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
+import org.apache.iceberg.rest.requests.CreateTableRequest;
+import org.apache.iceberg.rest.requests.CreateViewRequest;
+import org.apache.iceberg.rest.requests.RegisterTableRequest;
+import org.apache.iceberg.rest.requests.RenameTableRequest;
+import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
+import org.apache.iceberg.rest.responses.ConfigResponse;
+import org.apache.iceberg.rest.responses.LoadTableResponse;
+import org.apache.iceberg.rest.responses.LoadViewResponse;
+import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse;
+import org.apache.polaris.service.types.CommitTableRequest;
+import org.apache.polaris.service.types.CommitViewRequest;
+import org.apache.polaris.service.types.NotificationRequest;
+
+/**
+ * Event records for Iceberg REST Catalog operations. Each operation has
corresponding "Before" and
+ * "After" event records.
+ */
+public class IcebergRestCatalogEvents {
Review Comment:
Agreed, included in the next revision!
--
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
