dimas-b commented on code in PR #3456:
URL: https://github.com/apache/polaris/pull/3456#discussion_r2699740954


##########
runtime/service/src/main/java/org/apache/polaris/service/events/AttributeMap.java:
##########
@@ -18,11 +18,13 @@
  */
 package org.apache.polaris.service.events;
 
+import jakarta.enterprise.context.RequestScoped;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 
 /** A type-safe container for event attributes. This class is mutable and not 
thread-safe! */
+@RequestScoped

Review Comment:
   just thinking: `@RequestScoped` does not necessarily mean that the bean is 
accessed only by one thread.
   
   That said, current usage seems to be ok wrt concurrent access.



##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -183,6 +185,7 @@ public class IcebergCatalogHandler extends CatalogHandler 
implements AutoCloseab
   private final ReservedProperties reservedProperties;
   private final CatalogHandlerUtils catalogHandlerUtils;
   private final StorageAccessConfigProvider storageAccessConfigProvider;
+  private final AttributeMap attributeMap;

Review Comment:
   nit: maybe `EventAttributeMap`? When viewed in a context where the `import` 
statement is not visible, it is not obvious what the "attributes" are about 🤔 



##########
CHANGELOG.md:
##########
@@ -77,6 +77,7 @@ request adding CHANGELOG notes for breaking (!) changes and 
possibly other secti
 - (Before/After)UpdateTableEvent is emitted for all table updates within a 
transaction.
 - Added KMS options to Polaris CLI
 - Changed from Poetry to UV for Python package management 
+- Flattened Events hierarchy and introduced EventAttributes for all events. 
Additionally, `AttributeMap` is now exposed as a RequestScoped bean to allow 
intermediate event attributes to be added to events.

Review Comment:
   nit: `AttributeMap` is a low-level impl. detail... I'm not sure end user 
need to worry about it when reading the change log 🤔 



-- 
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]

Reply via email to