adnanhemani commented on code in PR #3195:
URL: https://github.com/apache/polaris/pull/3195#discussion_r2600701599
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -597,11 +615,30 @@ public Response commitTransaction(
polarisEventListener.onBeforeCommitTransaction(
new IcebergRestCatalogEvents.BeforeCommitTransactionEvent(
eventMetadataFactory.create(), catalogName,
commitTransactionRequest));
+ for (UpdateTableRequest req : commitTransactionRequest.tableChanges()) {
+ polarisEventListener.onBeforeUpdateTable(
+ new BeforeUpdateTableEvent(
+ eventMetadataFactory.create(),
+ catalogName,
+ req.identifier().namespace(),
+ req.identifier().name(),
+ req));
+ }
Response resp =
delegate.commitTransaction(prefix, commitTransactionRequest,
realmContext, securityContext);
polarisEventListener.onAfterCommitTransaction(
new IcebergRestCatalogEvents.AfterCommitTransactionEvent(
eventMetadataFactory.create(), catalogName,
commitTransactionRequest));
+ for (UpdateTableRequest req : commitTransactionRequest.tableChanges()) {
+ polarisEventListener.onAfterUpdateTable(
+ new AfterUpdateTableEvent(
+ eventMetadataFactory.create(),
+ catalogName,
+ req.identifier().namespace(),
+ req.identifier().name(),
+ req,
+ null));
Review Comment:
> My personal preference would be to remove LoadTableResponse from
AfterUpdateTableEvent in this PR (in all cases) and re-add when we can provide
it in all contexts.
This is fair too. I was trying to reduce the amount of changes to the event
structure itself and use the fact that this is null (only in the case of
transactions, to be clear) to place urgency in trying to fix #3209. I'll make
the change for the Javadoc - let me know if removing the LoadTableResponse is a
blocking concern for you. Else, I'd prefer to keep this approach the way it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
