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 >>>> >>>
