adnanhemani commented on code in PR #2765:
URL: https://github.com/apache/polaris/pull/2765#discussion_r2409030842
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergRestCatalogEventServiceDelegator.java:
##########
@@ -275,7 +275,8 @@ public Response loadTable(
realmContext,
securityContext);
polarisEventListener.onAfterLoadTable(
- new AfterLoadTableEvent(catalogName, namespaceObj, (LoadTableResponse)
resp.getEntity()));
+ new AfterLoadTableEvent(
+ catalogName, namespaceObj, table, (LoadTableResponse)
resp.getEntity()));
Review Comment:
Request POJOs don't always have all the fields unfortunately either. I get
the point though and agree with it generally.
My thought process has been to ensure that all information regarding what
was queried or changed as part of the request should be available in one way or
another in the After event using the following guidelines:
* If the information is regarding a Polaris entity, then the entity's
hierarchy should be in distinct fields within the event.
* If the request is for a modification of sorts, then the response object
should be placed as-is in the After event.
* All other relevant objects should be placed as similarly to the request
object into the event.
I know this isn't being followed across the board - but does this make
sense? Happy to make changes where this isn't the case or look into a refactor
if you believe in different guidelines.
--
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]
