Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2989892339 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: Thanks all! i will update the title and code, and send out a vote -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2989664098 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: Let's go with `Polaris-Generic-Table-Access-Delegation` -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
jbonofre commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2989487167 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I also have a preference for `Polaris-Generic-Table-Access-Delegation` for the completness. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
flyrain commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2989471260 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: The `Polaris-Generic-Table-Access-Delegation` is more accurate, but either `Polaris-Generic-Table-Access-Delegation` or `Polaris-Access-Delegation` works for me. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2986345119 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I think the direction of the “generic table” concept is still evolving, and it has the potential to expand beyond structured data to also cover unstructured data. From my perspective, the naming carries important implications. If we call it generic-table-access-delegation, it suggests that the header is scoped specifically to generic table APIs and applies only to entities managed (if generic table api covers views in the future, it also applies) within that context. On the other hand, polaris-access-delegation implies a broader, cross-cutting concept within Polaris that could be reused by future APIs. As Yufei mentioned, this could also extend to use cases like Iceberg tables. Since the overall direction is still not fully defined, I’m fine with either name for now, might more prefer to use generic-table prefix to restrict the scope, and let things evolve. However, I want to make sure we’re aligned on the underlying concept and intended scope before finalizing it. @dimas-b @flyrain @adutra please let me know what is your preference. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2986345119 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I think the direction of the “generic table” concept is still evolving, and it has the potential to expand beyond structured data to also cover unstructured data. From my perspective, the naming carries important implications. If we call it generic-table-access-delegation, it suggests that the header is scoped specifically to generic table APIs and applies only to entities managed within that context. On the other hand, polaris-access-delegation implies a broader, cross-cutting concept within Polaris that could be reused by future APIs. As Yufei mentioned, this could also extend to use cases like Iceberg tables. Since the overall direction is still not fully defined, I’m fine with either name for now, might more prefer to use generic-table prefix to restrict the scope, and let things evolve. However, I want to make sure we’re aligned on the underlying concept and intended scope before finalizing it. @dimas-b @flyrain @adutra please let me know what is your preference. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2986345119 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I think the direction of the “generic table” concept is still evolving, and it has the potential to expand beyond structured data to also cover unstructured data. From my perspective, the naming carries important implications. If we call it generic-table-access-delegation, it suggests that the header is scoped specifically to generic table APIs and applies only to entities managed within that context. On the other hand, polaris-access-delegation implies a broader, cross-cutting concept within Polaris that could be reused by future APIs. As Yufei mentioned, this could also extend to use cases like Iceberg tables. Since the overall direction is still not fully defined, I’m fine with either name for now, might more prefer to use generic-table prefix to restrict the scope. However, I want to make sure we’re aligned on the underlying concept and intended scope before finalizing it. ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I think the direction of the “generic table” concept is still evolving, and it has the potential to expand beyond structured data to also cover unstructured data. From my perspective, the naming carries important implications. If we call it generic-table-access-delegation, it suggests that the header is scoped specifically to generic table APIs and applies only to entities managed within that context. On the other hand, polaris-access-delegation implies a broader, cross-cutting concept within Polaris that could be reused by future APIs. As Yufei mentioned, this could also extend to use cases like Iceberg tables. Since the overall direction is still not fully defined, I’m fine with either name for now, might more prefer to use generic-table prefix to restrict the scope, and let things evolve. However, I want to make sure we’re aligned on the underlying concept and intended scope before finalizing it. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2986358970 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. + + +When `vended-credentials` is included, the server may return scoped Review Comment: sorry, i was just trying to use a wording that is more neural. i updated to use *must* to be more clear. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2986353336 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. + + +When `vended-credentials` is included, the server may return scoped +storage credentials in the `storage-access-configs` field of the response. + required: false + schema: +type: string +enum: Review Comment: Yes, enum can only be one of them, the iceberg spec description is not accurate, i updated the description to be more accurate. ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. Review Comment: i think we should be consistent on this across polaris, i updated the description to be more clear ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I think the direction of the “generic table” concept is still evolving, and it has the potential to expand beyond structured data to also cover unstructured data. From my perspective, the naming carries important implications. If we call it generic-table-access-delegation, it suggests that the header is scoped specifically to generic table APIs and applies only to entities managed within that context. On the other hand, polaris-access-delegation implies a broader, cross-cutting concept within Polaris that could be reused by future APIs. As Yufei mentioned, this could also extend to use cases like Iceberg tables. Since the overall direction is still not fully defined, I’m fine with either name for now. However, I want to make sure we’re aligned on the underlying concept and intended scope before finalizing it. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
flyrain commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2985091766 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: It is shorter. I think it's a good name. My only concern is that whether Polaris should honor it for Iceberg tables, when clients send `Polaris-Access-Delegation`. If we are OK with it, I'm onboard. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2984618767 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: Re: prefix - "Polaris" is the project that owns the API spec, so using the project name as the namespace for headers seems logical. Using "Generic" as the prefix is too generic :wink: `Polaris-Generic-Table-Access-Delegation` would work, from my POV, though. My personal preference (non-binding) is with `Polaris-Access-Delegation` because it is shorter and could be extended to other APIs owned by Polaris. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2984618767 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: Re: prefix - "Polaris" is the project that owns the API spec, so using the project name as the namespace for headers seems logical. Using "Generic" as the prefix is too generic :wink: `Polaris-Generic-Table-Access-Delegation` would work, from my POV, though. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2984608717 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: I meant "view" as something that is not a table. If the Generic Tables API is mean only for tables, please ignore my comment :) -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
flyrain commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2984481518 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. + + +When `vended-credentials` is included, the server may return scoped +storage credentials in the `storage-access-configs` field of the response. + required: false + schema: +type: string +enum: Review Comment: +1 on it. I think it shouldn't support comma-separated list -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
flyrain commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2984475915 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: > It could apply to views (hypothetically) too. What do you mean by view? Iceberg view should be covered by the Iceberg parameters. And if there is generic table view, it would be covered by `Generic-Table-Access-Delegation` as well. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
flyrain commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2984428242 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: +1 on removing the "X-" prefix. Since it is a `generic-table-data-access`, maybe `Generic-Table-Access-Delegation` makes more sense. Besides, if any other catalogs implements the generic table REST spec, `Polaris-Access-Delegation` would not be an appropriate name. WDYT? -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2982052963 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: Even HTTP no longer encourages using the `X-` prefix in headers, AFAIK. I'd propose `Polaris-Access-Delegation`. It could apply to views (hypothetically) too. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2982074069 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. + + +When `vended-credentials` is included, the server may return scoped Review Comment: Re: `may return` - do you mean that the server is not allowed to return credentials in `storage-access-configs` when this header value is not set? ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. + + +When `vended-credentials` is included, the server may return scoped +storage credentials in the `storage-access-configs` field of the response. + required: false + schema: +type: string +enum: Review Comment: `enum` means exactly one of many values can be specified, right? How can it work for a comma-separated list (line 194)? ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation + in: header + description: > +Optional signal to the server that the client supports delegated access +via a comma-separated list of access mechanisms. The server may choose +to supply access via any or none of the requested mechanisms. Review Comment: Re: `none` - in the Polaris IRC impl., if the client requests credential vending, but the server cannot provide them, the server will return an error response. Do we want the server to silently ignore credential vending requests in the Generic Tables API? -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
dimas-b commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2982052963 ## spec/polaris-catalog-apis/generic-tables-api.yaml: ## @@ -182,6 +186,26 @@ components: type: string example: "sales" +generic-table-data-access: + name: X-Generic-Table-Access-Delegation Review Comment: Even HTTP no long encourages using the `X-` prefix in headers, AFAIK. I'd propose `Polaris-Access-Delegation`. It could apply to views (hypothetically) too. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
adutra commented on PR #4043: URL: https://github.com/apache/polaris/pull/4043#issuecomment-4116004262 Shouldn't we use the `X-Polaris` prefix? -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
jbonofre commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2979544472 ## runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java: ## @@ -72,6 +72,7 @@ public Response createGenericTable( String prefix, String namespace, CreateGenericTableRequest createGenericTableRequest, + String xGenericTableAccessDelegation, Review Comment: Totally fine. Thanks ! -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
gh-yzou commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2979520592 ## runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java: ## @@ -72,6 +72,7 @@ public Response createGenericTable( String prefix, String namespace, CreateGenericTableRequest createGenericTableRequest, + String xGenericTableAccessDelegation, Review Comment: Yes, this isn’t implemented yet. The initial goal of this PR was just to update the API spec, but the build started failing since adding new parameters affects the generated interface. I’ll follow up with additional PRs to implement the credential vending support. -- 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]
Re: [PR] Add X-Generic-Table-Access-Delegation header in the Generic Table API spec for credential vending [polaris]
jbonofre commented on code in PR #4043: URL: https://github.com/apache/polaris/pull/4043#discussion_r2979118606 ## runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogAdapter.java: ## @@ -72,6 +72,7 @@ public Response createGenericTable( String prefix, String namespace, CreateGenericTableRequest createGenericTableRequest, + String xGenericTableAccessDelegation, Review Comment: I don't see this argument used anywhere. I guess it's a "preparation" PR, just for the header, not yet actually used. -- 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]
