sungwy commented on code in PR #4356:
URL: https://github.com/apache/polaris/pull/4356#discussion_r3438887556
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java:
##########
@@ -78,6 +85,8 @@ public RealmContext realmContext() {
public abstract PolarisAuthorizer authorizer();
+ public abstract AuthorizationState authorizationState();
Review Comment:
Hi @flyrain thanks for the detailed review.
I see why it reads that way, but I think the intended contract is different.
`AuthorizationState` is meant to be request-scoped state that the
`PolarisAuthorizer` can manipulate throughout the request. This is basically a
renamed version of the “auth call context” idea we discussed in the [Polaris
Community Sync
1/27/2026](https://docs.google.com/document/d/1C_SSaZH1i83UUGXrnVBur1fR_FHKYWZ75ISFfcb3kns/edit?tab=t.0),
which allows us to avoid having to do a large refactoring of the
Persistence/Resolution layer to reach the same outcome today of skipping RBAC
resolution for external authorizers.
The write-once behavior per request is intentional to me. For a given
request, we should know the full path of the resource we want to resolve up
front. If we try to swap the manifest halfway through the request, I’d rather
fail loudly than silently authorize/execute against a different resolved view.
I agree the current duplicate storage is awkward: the handler field and
`AuthorizationState` point to the same thing. But I think that becomes a
supporting case for a refactoring of the Resolver/Persistence layer, rather
than making `AuthorizationState` a disposable per-call wrapper.
--
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]
