dimas-b commented on code in PR #3724:
URL: https://github.com/apache/polaris/pull/3724#discussion_r2799499598
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java:
##########
@@ -144,7 +145,7 @@ public enum PolarisAuthorizableOperation {
DROP_VIEW(VIEW_DROP),
VIEW_EXISTS(VIEW_LIST),
RENAME_VIEW(VIEW_DROP, EnumSet.of(VIEW_LIST, VIEW_CREATE)),
- REPORT_METRICS(EnumSet.noneOf(PolarisPrivilege.class)),
+ REPORT_METRICS(TABLE_REPORT_METRICS),
Review Comment:
Having a separate permission for metrics seems more flexible and more
coherent to me... mainly because the behaviour of this endpoint is very
different from the endpoints engaged in reading a table.
That said, considering Scan and Commit Reports (the only kinds supported
ATM), it probably makes sense to allow anyone who can query a table to able
submit scan reports.
If I understand correctly, `TABLE_REPORT_METRICS` is implied (subsumed) by
`CATALOG_MANAGE_CONTENT`, `TABLE_FULL_METADATA`. This set likely ensures that
most table readers is able able to submit metrics already.
Metrics Reports are supported only since 1.3.0 (which is a very recent
release), so given the above, I would not expect major breakage on the client
side due to introducing the new permission.
My preference would be to merge the current PR "as 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]
