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