adnanhemani commented on code in PR #2480:
URL: https://github.com/apache/polaris/pull/2480#discussion_r2331371789
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -103,8 +202,29 @@ public Response createTable(
String accessDelegationMode,
RealmContext realmContext,
SecurityContext securityContext) {
- return delegate.createTable(
- prefix, namespace, createTableRequest, accessDelegationMode,
realmContext, securityContext);
+ String catalogName = prefixParser.prefixToCatalogName(realmContext,
prefix);
+ if (!createTableRequest.stageCreate()) {
Review Comment:
My understanding is that when a table is stageCreated, then no actual table
is made - and therefore there is no new (meta)data created. In that case, I
don't think it makes sense to emit a `AfterCreateTableEvent` if a table was not
truly created.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -212,8 +384,12 @@ public Response listViews(
Integer pageSize,
RealmContext realmContext,
SecurityContext securityContext) {
- return delegate.listViews(
- prefix, namespace, pageToken, pageSize, realmContext, securityContext);
+ String catalogName = prefixParser.prefixToCatalogName(realmContext,
prefix);
+ polarisEventListener.onBeforeListViews(new
BeforeListViewsEvent(catalogName, namespace));
Review Comment:
This is a good callout - we should switch this to use `Namespace` objects
instead. Doing in the next revision.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -273,10 +477,21 @@ public Response replaceView(
CommitViewRequest commitViewRequest,
RealmContext realmContext,
SecurityContext securityContext) {
- return delegate.replaceView(
- prefix, namespace, view, commitViewRequest, realmContext,
securityContext);
+ String catalogName = prefixParser.prefixToCatalogName(realmContext,
prefix);
+ polarisEventListener.onBeforeReplaceView(
+ new BeforeReplaceViewEvent(catalogName, namespace, view,
commitViewRequest));
+ Response resp =
+ delegate.replaceView(
+ prefix, namespace, view, commitViewRequest, realmContext,
securityContext);
+ polarisEventListener.onAfterReplaceView(
+ new AfterReplaceViewEvent(
+ catalogName, namespace, view, (LoadViewResponse)
resp.getEntity()));
+ return resp;
}
+ /**
+ * Table Committed Events are already instrumented at a more granular level
than the API itself.
+ */
Review Comment:
The TableCommittedEvent was previously merged and there are no changes to it
as part of this PR. The idea is that all table commits that happened as part of
the same Polaris API call (through the `commitTransaction` API) will already be
tied together through the same request ID and so a new Event for this API is
not explicitly required.
--
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
