dimas-b commented on code in PR #3924:
URL: https://github.com/apache/polaris/pull/3924#discussion_r2898565747
##########
site/content/in-dev/unreleased/proposals/observability-rest-api.md:
##########
@@ -1072,7 +1072,49 @@ components:
## 7. Implementation Notes
-### 7.1 Database Queries
+### 7.1 Prerequisite: Extend Event Persistence Layer
+
+> **Important:** The current `PolarisPersistenceEventListener` in Apache
Polaris only persists **two event types**: `AFTER_CREATE_TABLE` and
`AFTER_CREATE_CATALOG`. All other events are ignored. For the Events API to be
useful, the persistence layer must be extended to capture all relevant mutation
events.
+
+#### Current State
+
+The existing event listener (`PolarisPersistenceEventListener.java`) has a
limited switch statement:
+
+```java
+public void onEvent(PolarisEvent event) {
+ switch (event.type()) {
+ case AFTER_CREATE_TABLE -> handleAfterCreateTable(event);
+ case AFTER_CREATE_CATALOG -> handleAfterCreateCatalog(event);
+ default -> {
+ // Other events not handled by this listener
+ }
+ }
+}
+```
+
+#### Required Changes
+
+The persistence layer needs to be extended to capture all `AFTER_*` mutation
events that should be exposed via the Events API:
+
+| Category | Events to Add |
+|----------|---------------|
+| **Table Operations** | `AFTER_UPDATE_TABLE`, `AFTER_DROP_TABLE`,
`AFTER_RENAME_TABLE`, `AFTER_REGISTER_TABLE` |
+| **View Operations** | `AFTER_CREATE_VIEW`, `AFTER_DROP_VIEW`,
`AFTER_REPLACE_VIEW`, `AFTER_RENAME_VIEW` |
+| **Namespace Operations** | `AFTER_CREATE_NAMESPACE`,
`AFTER_UPDATE_NAMESPACE_PROPERTIES`, `AFTER_DROP_NAMESPACE` |
+| **Catalog Operations** | `AFTER_DELETE_CATALOG`, `AFTER_UPDATE_CATALOG` |
+| **Principal/Role Management** | `AFTER_CREATE_PRINCIPAL`,
`AFTER_DELETE_PRINCIPAL`, `AFTER_ROTATE_CREDENTIALS`,
`AFTER_CREATE_PRINCIPAL_ROLE`, `AFTER_DELETE_PRINCIPAL_ROLE`,
`AFTER_CREATE_CATALOG_ROLE`, `AFTER_DELETE_CATALOG_ROLE` |
+| **Grant Operations** | `AFTER_ADD_GRANT_TO_CATALOG_ROLE`,
`AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE`, `AFTER_ASSIGN_PRINCIPAL_ROLE`,
`AFTER_REVOKE_PRINCIPAL_ROLE`, `AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE`,
`AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE` |
+| **Policy Operations** | `AFTER_CREATE_POLICY`, `AFTER_UPDATE_POLICY`,
`AFTER_DROP_POLICY`, `AFTER_ATTACH_POLICY`, `AFTER_DETACH_POLICY` |
+
+**Note:** Read-only operations (`AFTER_LOAD_*`, `AFTER_LIST_*`, `AFTER_GET_*`,
`AFTER_CHECK_EXISTS_*`) should **not** be persisted for the Events API as they
do not represent catalog mutations. However, deployments requiring read audit
may choose to persist these separately.
+
+#### Implementation Approach
+
+1. **Phase 1**: Extend `PolarisPersistenceEventListener` to handle all
Iceberg-standard operations (tables, views, namespaces)
Review Comment:
Definitely what to persist is outside the scope of this API proposal, which
is about who to retrieve what has been persisted :)
##########
site/content/in-dev/unreleased/proposals/observability-rest-api.md:
##########
@@ -1072,7 +1072,49 @@ components:
## 7. Implementation Notes
-### 7.1 Database Queries
+### 7.1 Prerequisite: Extend Event Persistence Layer
+
+> **Important:** The current `PolarisPersistenceEventListener` in Apache
Polaris only persists **two event types**: `AFTER_CREATE_TABLE` and
`AFTER_CREATE_CATALOG`. All other events are ignored. For the Events API to be
useful, the persistence layer must be extended to capture all relevant mutation
events.
+
+#### Current State
+
+The existing event listener (`PolarisPersistenceEventListener.java`) has a
limited switch statement:
+
+```java
+public void onEvent(PolarisEvent event) {
+ switch (event.type()) {
+ case AFTER_CREATE_TABLE -> handleAfterCreateTable(event);
+ case AFTER_CREATE_CATALOG -> handleAfterCreateCatalog(event);
+ default -> {
+ // Other events not handled by this listener
+ }
+ }
+}
+```
+
+#### Required Changes
+
+The persistence layer needs to be extended to capture all `AFTER_*` mutation
events that should be exposed via the Events API:
+
+| Category | Events to Add |
+|----------|---------------|
+| **Table Operations** | `AFTER_UPDATE_TABLE`, `AFTER_DROP_TABLE`,
`AFTER_RENAME_TABLE`, `AFTER_REGISTER_TABLE` |
+| **View Operations** | `AFTER_CREATE_VIEW`, `AFTER_DROP_VIEW`,
`AFTER_REPLACE_VIEW`, `AFTER_RENAME_VIEW` |
+| **Namespace Operations** | `AFTER_CREATE_NAMESPACE`,
`AFTER_UPDATE_NAMESPACE_PROPERTIES`, `AFTER_DROP_NAMESPACE` |
+| **Catalog Operations** | `AFTER_DELETE_CATALOG`, `AFTER_UPDATE_CATALOG` |
+| **Principal/Role Management** | `AFTER_CREATE_PRINCIPAL`,
`AFTER_DELETE_PRINCIPAL`, `AFTER_ROTATE_CREDENTIALS`,
`AFTER_CREATE_PRINCIPAL_ROLE`, `AFTER_DELETE_PRINCIPAL_ROLE`,
`AFTER_CREATE_CATALOG_ROLE`, `AFTER_DELETE_CATALOG_ROLE` |
+| **Grant Operations** | `AFTER_ADD_GRANT_TO_CATALOG_ROLE`,
`AFTER_REVOKE_GRANT_FROM_CATALOG_ROLE`, `AFTER_ASSIGN_PRINCIPAL_ROLE`,
`AFTER_REVOKE_PRINCIPAL_ROLE`, `AFTER_ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE`,
`AFTER_REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE` |
+| **Policy Operations** | `AFTER_CREATE_POLICY`, `AFTER_UPDATE_POLICY`,
`AFTER_DROP_POLICY`, `AFTER_ATTACH_POLICY`, `AFTER_DETACH_POLICY` |
+
+**Note:** Read-only operations (`AFTER_LOAD_*`, `AFTER_LIST_*`, `AFTER_GET_*`,
`AFTER_CHECK_EXISTS_*`) should **not** be persisted for the Events API as they
do not represent catalog mutations. However, deployments requiring read audit
may choose to persist these separately.
+
+#### Implementation Approach
+
+1. **Phase 1**: Extend `PolarisPersistenceEventListener` to handle all
Iceberg-standard operations (tables, views, namespaces)
Review Comment:
Definitely what to persist is outside the scope of this API proposal, which
is about how to retrieve what has been persisted :)
--
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]
