dennishuo commented on code in PR #211:
URL: https://github.com/apache/polaris/pull/211#discussion_r1731849454
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java:
##########
@@ -171,28 +173,39 @@ public Response updateProperties(
.build();
}
+ private EnumSet<AccessDelegationMode> parseAccessDelegationModes(String
accessDelegationMode) {
+ EnumSet<AccessDelegationMode> delegationModes =
+ AccessDelegationMode.fromProtocolValuesList(accessDelegationMode);
+ if (!delegationModes.isEmpty() &&
!delegationModes.contains(VENDED_CREDENTIALS)) {
Review Comment:
It seems like there will be some need to reconcile "strict intent" vs
"hints" better in the future, since right now a lot of the Iceberg spec is
open-ended about interpretation -- for example, the spec currently says:
The server may choose to supply access via any or none of the requested
mechanisms.
This also relates to https://github.com/apache/iceberg/pull/10359 in that
some behaviors currently can only "guess" at the caller's intent.
I agree that any hard-coding of "true" is probably not good baggage to carry
forward. I would say though that extrapolating from
https://github.com/apache/polaris/pull/77 and how the Tabular "demo REST docker
image" provides hooks for directly returning the server-wide application
credentials in credential-vending, maybe it's worth providing some
extensibility here.
What if we extract a server-level configuration parameter or two:
- SUPPORTED_ACCESS_DELEGATION_MODES=vended-credentials,...
- ERROR_ON_FAILED_NEGOTIATION_FOR_ACCESS_DELEGATION_MODE=true
The first one would just be a set-intersection between the list of modes the
client wants and the set that the server claims to accept. I suppose the later
mapping of declared modes to access-delegation impl will be something to add in
the future once we actually need to plumb it through to
`PolarisCatalogHandlerWrapper` to do different things for different modes.
The second one will dictate whether the server should be in "strict" mode
and return an error response, or instead be permissive and just log a warning
(or if there was a way to return helpful warnings in Iceberg REST responses
that'd be great).
To the last point I wonder if we should also think of proposing some
standard scaffolding for "warning notices" in REST responses that
client-libraries can start to support to display to a caller in whatever way is
most natural for the client-library. I know cloud providers have gone through
this exercise before so that individual service providers can have a standard
way to start providing deprecation notices, etc.
--
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]
