sungwy commented on code in PR #4356:
URL: https://github.com/apache/polaris/pull/4356#discussion_r3456702890
##########
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 I don’t currently have a concrete second attribute in mind for
`AuthorizationState`. The main state I care about here is the
`PolarisResolutionManifest`, because it represents the resolved view we use
across authz and execution.
The forward-looking part is that this was meant to preserve today’s
resolution semantics while moving the decision of *what to resolve* into the
`PolarisAuthorizer`. Longer term, I still think a larger Persistence/Resolution
refactor is probably the cleaner direction, so handlers and authorizers are
less coupled to a callsite-managed `PolarisResolutionManifest` directly.
Keeping `AuthorizationState` request-scoped felt like a small step in that
direction, but I agree it is not the full design.
Given that the larger refactor has not been designed yet, I’m okay relaxing
this for now and treating `AuthorizationState` as a per-resolution wrapper
around the manifest. That keeps this PR focused on moving resolution choice
into the authorizer, while leaving request-scoped persistence/resolution state
for a more concrete follow-up design.
The tradeoff is that we lose the request-lifetime guard and keep
responsibility for resolved-view consistency at the call sites, which is
basically where it already lives today.
@flyrain , @dimas-b - does that sound like a good way to move this PR
forward?
--
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]
