Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Support endpoints field in the config endpoint #316

Open
flyrain opened this issue Sep 25, 2024 · 6 comments · May be fixed by #383
Open

[FEATURE REQUEST] Support endpoints field in the config endpoint #316

flyrain opened this issue Sep 25, 2024 · 6 comments · May be fixed by #383
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@flyrain
Copy link
Contributor

flyrain commented Sep 25, 2024

Is your feature request related to a problem? Please describe.

No response

Describe the solution you'd like

The endpoints field was introduced in Iceberg REST spec recently to describe the server capabilities.
https://github.com/apache/iceberg/blob/c0d73f4ef5c16401bdfd62e1745faf2fbbf62177/open-api/rest-catalog-open-api.yaml#L150

Describe alternatives you've considered

No response

Additional context

No response

@flyrain flyrain added the enhancement New feature or request label Sep 25, 2024
@flyrain flyrain added the good first issue Good for newcomers label Sep 25, 2024
@danielhumanmod
Copy link

Hi @flyrain , I am interested in working on this feature :)

@danielhumanmod
Copy link

And I have a question about the rule for adding API endpoints: do we mainly based on the access control privileges?

For example:

  • if a user with NAMESPACE_LIST privilege, add these into endpoints
    • GET /v1/{prefix}/namespaces
    • GET /v1/{prefix}/namespaces/{namespace}
    • GET /v1/{prefix}/namespaces/{namespace}/tables
    • GET /v1/{prefix}/namespaces/{namespace}/tables/{table}
  • if a user with NAMESPACE_CREATE, add this into endpoints
    • POST /v1/{prefix}/namespaces
  • if a user with NAMESPACE_DROP , add this into endpoints
    • `DELETE /v1/{prefix}/namespaces'

@flyrain
Copy link
Contributor Author

flyrain commented Oct 16, 2024

I don't believe it emits endpoints based on privileges; it should simply list all available endpoints. Even if a client lacks the privilege for a specific endpoint, they would still receive the full capabilities of the server.

@danielhumanmod
Copy link

danielhumanmod commented Oct 16, 2024

I don't believe it emits endpoints based on privileges; it should simply list all available endpoints. Even if a client lacks the privilege for a specific endpoint, they would still receive the full capabilities of the server.

Thanks for the clarification! That approach makes a lot more sense, my earlier approach was overcomplicating it.

@danielhumanmod
Copy link

Hi @flyrain, I created a draft PR as the POC of this feature. However, I’d like your suggestion on one thing: the endpoints-related feature in Iceberg hasn’t been released yet (current version 1.6.1) and is planned for 1.7.0. I’ve temporarily extended some necessary classes as a workaround. Would you suggest we proceed with the current approach, or would it be better to pause development and continue once 1.7.0 is released and integrated with Polaris?

@flyrain
Copy link
Contributor Author

flyrain commented Oct 18, 2024

I'd recommend to wait for iceberg 1.7.0. It's not urgent as clients probably also wait for 1.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants