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
