I’m OK with using a plain string for the endpoint ID, as described in doc[1]. However, I’ve been thinking about how we can make this more flexible, especially since we’ve had quite a few discussions about granularity.
For instance, if we expose a bit more of the API specification, we might not need some of the feature flags. It might be worthwhile to consider adopting the OpenAPI path object schema[2] if we decide to add more structure. Since Iceberg is already using OpenAPI to define the REST specification and will continue to do so, this approach could make future extensions easier. Looking forward to your thoughts on this! Ref: 1. API Endpoints <https://learn.openapis.org/specification/paths.html#api-endpoints> 2. Path Object <https://spec.openapis.org/oas/v3.1.0#paths-object> Yufei On Fri, Aug 16, 2024 at 8:58 AM karuppayya <karuppayya1...@gmail.com> wrote: > Can we consider adding `OPTIONS` verb to the resource paths, as part of > the spec? > That way the endpoint discovery endpoint could return only the list of > supported endpoints, without the verbs. > > `OPTIONS` on the resource path can return the list of all supported verbs, > and also other information regarding API usage > > - Karuppayya > > On Fri, Aug 16, 2024 at 7:34 AM Dmitri Bourlatchkov > <dmitri.bourlatch...@dremio.com.invalid> wrote: > >> Sorry for a bit of back-tracking, but I'd like to clarify the meaning of >> those endpoint strings. >> >> I initially assumed that the strings would need to be parsed into >> components (verb / path) for use in runtime. My suggestion for using a JSON >> representation was meant to make the parsing more standard (i.e. avoid >> custom format). >> >> After reading the impl. PR [1] I think parsing is actually _not_ >> necessary. The whole endpoint string is basically another form of an >> endpoint "ID". >> >> If we agree that the values in the new endpoints config list should be >> treated as opaque IDs, I think it should be fine to use the simple string >> representation. >> >> WDYT? >> >> Thanks, >> Dmitri. >> >> [1] https://github.com/apache/iceberg/pull/10929 >> >> On Fri, Aug 16, 2024 at 4:43 AM Eduard Tudenhöfner < >> etudenhoef...@apache.org> wrote: >> >>> If we really want to add more structure to the JSON representation, then >>> I would probably prefer what Dmitri suggested in the doc as I think { >>> "GET": {}, "POST": {} } looks a bit weird: >>> >>> "endpoints":[ >>> {"verb": "GET","path": "/v1/{prefix}/namespaces/{namespace}"}, >>> {"verb": "GET","path": "/v1/{prefix}/namespaces"}, >>> {"verb": "POST","path": "/v1/{prefix}/namespaces"} >>> ] >>> >>> What do people think of that? >>> >>> >>> >>> On Fri, Aug 16, 2024 at 8:13 AM Walaa Eldin Moustafa < >>> wa.moust...@gmail.com> wrote: >>> >>>> Thank you Eduard for sharing this version of the proposal. Looks >>>> simple, functional, and extensible. >>>> >>>> On Thu, Aug 15, 2024 at 1:10 PM Ryan Blue <b...@databricks.com.invalid> >>>> wrote: >>>> >>>>> I think I'm fine either way. I lean toward the simplicity of the >>>>> strings in the proposal but would not complain if we went with Yufei's >>>>> suggestion. >>>>> >>>>> On Thu, Aug 15, 2024 at 12:12 PM Yufei Gu <flyrain...@gmail.com> >>>>> wrote: >>>>> >>>>>> The current proposal lists endpoints as plain strings, and I still >>>>>> believe we could make things a bit smoother by adding some structure to >>>>>> them. Here's the example if the previous one throws you off. >>>>>> >>>>>> *Before:* >>>>>> >>>>>> "GET /v1/{prefix}/namespaces","POST /v1/{prefix}/namespaces","GET >>>>>> /v1/{prefix}/namespaces/{namespace}","DELETE >>>>>> /v1/{prefix}/namespaces/{namespace}" >>>>>> >>>>>> *After:* >>>>>> >>>>>> {"/v1/{prefix}/namespaces": { "GET": {}, "POST": {} } >>>>>> },{"/v1/{prefix}/namespaces/{namespace}": { "GET": {}, "DELETE": {} } } >>>>>> >>>>>> I think this approach would make the definitions more >>>>>> machine-readable and easier to expand in the future. It's always good to >>>>>> plan for potential changes, and this structure should help us adapt >>>>>> without >>>>>> much hassle. >>>>>> Plus, we could use a trimmed OpenAPI schema for this, so we wouldn’t >>>>>> need to create a new schema from scratch. >>>>>> >>>>>> Yufei >>>>>> >>>>>> >>>>>> On Thu, Aug 15, 2024 at 11:02 AM Jack Ye <yezhao...@gmail.com> wrote: >>>>>> >>>>>>> > But I propose to use a trimmed openAPI's format directly. >>>>>>> Looking at the example, this feels quite complicated to me. >>>>>>> >>>>>>> > For example, it is easier if we want to include operationID >>>>>>> I don't think we need to consider accommodating both, since >>>>>>> operationId is an alternative to "<HTTP VERB> <resource path from REST >>>>>>> spec>". >>>>>>> >>>>>>> > or adding feature flags, or adding parameters >>>>>>> For more advanced feature flags, those could likely directly go to >>>>>>> overrides just like today, but given we limited the scope to service >>>>>>> endpoint discovery, I think that is another discussion later. >>>>>>> >>>>>>> So the current proposed solution from Eduard still feels better to >>>>>>> me. And I think the argument for ambiguity is pretty strong so I am good >>>>>>> with the proposed approach to use "<HTTP VERB> <resource path from REST >>>>>>> spec>". >>>>>>> >>>>>>> -Jack >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Aug 15, 2024 at 9:46 AM Yufei Gu <flyrain...@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> +1 for the proposal. In terms of the format, the current solution >>>>>>>> is simple enough. But I propose to use a trimmed openAPI's format >>>>>>>> directly. >>>>>>>> It won't add much cost as we can just take the minimum fields we want. >>>>>>>> But >>>>>>>> it opens a window to extend it in the future. For example, it is >>>>>>>> easier if >>>>>>>> we want to include operationID, or adding feature flags, or adding >>>>>>>> parameters. Here is an example: >>>>>>>> >>>>>>>> { >>>>>>>> >>>>>>>> "resources": [ >>>>>>>> >>>>>>>> { >>>>>>>> >>>>>>>> "/v1/{prefix}/namespaces": >>>>>>>> >>>>>>>> { >>>>>>>> >>>>>>>> "GET": { >>>>>>>> >>>>>>>> "description": "List all namespaces" >>>>>>>> >>>>>>>> } >>>>>>>> >>>>>>>> }, >>>>>>>> >>>>>>>> { >>>>>>>> >>>>>>>> "POST": { >>>>>>>> >>>>>>>> "description": "Create a new namespace" >>>>>>>> >>>>>>>> } >>>>>>>> >>>>>>>> } >>>>>>>> >>>>>>>> }, >>>>>>>> >>>>>>>> { >>>>>>>> >>>>>>>> "path2": {} >>>>>>>> >>>>>>>> } >>>>>>>> >>>>>>>> ... >>>>>>>> >>>>>>>> ] >>>>>>>> >>>>>>>> } >>>>>>>> >>>>>>>> >>>>>>>> Yufei >>>>>>>> >>>>>>>> >>>>>>>> On Thu, Aug 15, 2024 at 8:47 AM Russell Spitzer < >>>>>>>> russell.spit...@gmail.com> wrote: >>>>>>>> >>>>>>>>> I'm on board for this proposal. I was in the off-mail chats and I >>>>>>>>> think this is probably our simplest approach going forward. >>>>>>>>> >>>>>>>>> On Thu, Aug 15, 2024 at 10:39 AM Dmitri Bourlatchkov >>>>>>>>> <dmitri.bourlatch...@dremio.com.invalid> wrote: >>>>>>>>> >>>>>>>>>> OpenAPI tool will WARN a lot if Operation IDs overlap. Generated >>>>>>>>>> code/html may also look odd in case of overlaps. >>>>>>>>>> >>>>>>>>>> All-in-all, I think the best practice is to define unique >>>>>>>>>> Operation IDs up front. >>>>>>>>>> >>>>>>>>>> For Iceberg REST API, the yaml file is the API definition, so it >>>>>>>>>> should not be a problem to ensure that Operation IDs are unique, I >>>>>>>>>> guess. >>>>>>>>>> >>>>>>>>>> Cheers, >>>>>>>>>> Dmitri. >>>>>>>>>> >>>>>>>>>> On Thu, Aug 15, 2024 at 11:32 AM Eduard Tudenhöfner < >>>>>>>>>> etudenhoef...@apache.org> wrote: >>>>>>>>>> >>>>>>>>>>> Hey Jack, >>>>>>>>>>> >>>>>>>>>>> thanks for the feedback. I replied in the doc but I can >>>>>>>>>>> reiterate my answer here too: The *path* is unique and required >>>>>>>>>>> so that feels more appropriate than requiring to have an optional >>>>>>>>>>> *operationId* in the OpenAPI spec. >>>>>>>>>>> Additionally, using the path is more straight-forward when we >>>>>>>>>>> introduce v2 endpoints, while you would have to make sure that all >>>>>>>>>>> *operationIds* are unique across endpoints (and I'm not sure if >>>>>>>>>>> OpenAPI tools actually enforce uniqueness). >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Thu, Aug 15, 2024 at 5:20 PM Jack Ye <yezhao...@gmail.com> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Eduard, >>>>>>>>>>>> >>>>>>>>>>>> In general I agree with this proposal, thanks for putting this >>>>>>>>>>>> up! Just one question (which I also added in the design), what are >>>>>>>>>>>> the >>>>>>>>>>>> thoughts behind using "<HTTP VERB> <resource path from REST >>>>>>>>>>>> spec>", vs >>>>>>>>>>>> using the operationId defined in the OpenAPI? >>>>>>>>>>>> >>>>>>>>>>>> The operationId approach definitely looks much cleaner to me, >>>>>>>>>>>> but (1) in OpenAPI it is not a requirement to define it, and (2) >>>>>>>>>>>> right now >>>>>>>>>>>> there are some inconsistent operationIds, for example UpdateTable >>>>>>>>>>>> is the >>>>>>>>>>>> operationId, but CommitTable is used for all request and response >>>>>>>>>>>> models. >>>>>>>>>>>> But these are all pretty solvable issues because we can enforce >>>>>>>>>>>> operationId >>>>>>>>>>>> to be required in the Iceberg spec, and fix it to be consistent, >>>>>>>>>>>> assuming >>>>>>>>>>>> nobody is taking a dependency on these operationIds right now. >>>>>>>>>>>> >>>>>>>>>>>> Personally speaking, I am pretty neutral on this topic, but >>>>>>>>>>>> curious what everyone thinks. >>>>>>>>>>>> >>>>>>>>>>>> Best, >>>>>>>>>>>> Jack Ye >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Wed, Aug 14, 2024 at 9:20 AM Eduard Tudenhöfner < >>>>>>>>>>>> etudenhoef...@apache.org> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Hey Dmitri, >>>>>>>>>>>>> >>>>>>>>>>>>> this proposal is the result of the community feedback from the >>>>>>>>>>>>> Capabilities proposal. Ultimately the capabilities turned out to >>>>>>>>>>>>> entail >>>>>>>>>>>>> more complexity than necessary and so this proposal solves the >>>>>>>>>>>>> core problem >>>>>>>>>>>>> while keeping complexity and spec changes to an absolute minimum. >>>>>>>>>>>>> >>>>>>>>>>>>> Eduard >>>>>>>>>>>>> >>>>>>>>>>>>> On Wed, Aug 14, 2024 at 5:15 PM Dmitri Bourlatchkov >>>>>>>>>>>>> <dmitri.bourlatch...@dremio.com.invalid> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi Eduard, >>>>>>>>>>>>>> >>>>>>>>>>>>>> How is this proposal related to the Server Capabilities >>>>>>>>>>>>>> discussion? >>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>> Dmitri. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Wed, Aug 14, 2024 at 5:14 AM Eduard Tudenhöfner < >>>>>>>>>>>>>> etudenhoef...@apache.org> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hey everyone, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I'd like to propose a way for REST servers to communicate to >>>>>>>>>>>>>>> clients what endpoints it supports via a new *endpoints* >>>>>>>>>>>>>>> field in the *CatalogConfig* of the *v1/config* endpoint. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This enables clients to make better decisions and clearly >>>>>>>>>>>>>>> signal that a particular endpoint isn’t supported. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I opened #10937 >>>>>>>>>>>>>>> <https://github.com/apache/iceberg/issues/10937> to track >>>>>>>>>>>>>>> the proposal in GH. Please find the proposal doc here >>>>>>>>>>>>>>> <https://docs.google.com/document/d/1krcIaLfxtBFDABU5ssLmf64zyHgE8BRncpGPIMTWlxo/edit?usp=sharing> >>>>>>>>>>>>>>> (estimated >>>>>>>>>>>>>>> read time: 5 minutes). The proposal requires a Spec change, >>>>>>>>>>>>>>> which can be >>>>>>>>>>>>>>> seen in #10928 >>>>>>>>>>>>>>> <https://github.com/apache/iceberg/pull/10928>. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Eduard >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>> >>>>> -- >>>>> Ryan Blue >>>>> Databricks >>>>> >>>>
