I think it's more straightforward to use the format from the existing proposal. That's unambiguous and seems easier to understand to me, rather than needing to refer to the spec to find out the details. The `UpdateTable` vs `CommitTable` discrepancy is a good example, and we could have names that are similar like using `CommitTables` for multi-table commits, which is off by just one letter.
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
