I think Robert's approach is a reasonable compromise here.

If we wanted a "per operation/endpoint" versioning, I think I'd prefer
Micah's OpenAPI spec based approach because it's more standardized, but I
feel adds a lot of client complexity.

-Dan



On Wed, Jun 26, 2024 at 6:59 AM Robert Stupp <sn...@snazy.de> wrote:

> (I think, compatibility deserves a separate thread - it's a "huge" topic)
>
> Based on experience, we decided on the following with Nessie:
>
>    - Unknown fields/attributes in a structure _DO_ cause
>    (de)serialization failures.
>    - "Stable API versions" - endpoint additions and/or added query
>    parameters and/or enhanced structures do _NOT_ require a new API version
>    (as in the endpoint's route/path).
>    - "Flexible spec versions" - new and updated "capabilities" however
>    might cause a bump in the "spec version" that the server announces in its
>    `getConfig` result.
>
> Adding new routes/paths may require new endpoint implementations on the
> server side, which can easily lead to a lot of (unnecessarily boilerplate)
> code. Using different routes/paths is justified if the API is changed
> "fundamentally". We call the "path component" (api/v1/..., api/v2/...) API
> version - the server indicates the minimum and maximum supported API
> version, in case a client wants to "upgrade". I recommend to _not_ bump the
> API version in the route/path if it's not really necessary.
>
> Regarding the requirement to fail on unknown attributes: Unknown
> attributes may contain important information. A client may send a newer
> version of a request object with an important new field, but the (older)
> server discards the new attribute. Think of an attribute that for example
> defines a "commit condition" that the client expects to be respected. "New"
> attributes must be omittable (e.g. don't serialize if null/default) -
> clients indicate the "usage" of an added attribute using some request
> attribute (for example: "boolean returnExtendedInformation").
>
> The list of capabilities can be indicated with included "spec versions",
> to tell clients which features/functionalities a server
> supports."Production" spec versions could start with 1, and "reserve" 0 for
> experimental/unsupported/poc kind of implementation. It could look like
> this:
>   capabilities: [
>     "table-spec/2,3",   // but not table-spec v1 here
>     "view-spec/1",
>     "table-api/1",
>     "view-api/1",
>     "udf-api/1",
>     "super-feature/2,4,6",   // but not spec versions 0,1,3,5,7+
>     ...
>   ]
> Incrementing a spec version in the list of capabilities doesn't break any
> client. We could also define a structure to describe each capability:
>   components:
>     schemas:
>       Capability:
>         name:
>           type: string
>           description: Name of the capability
>         versions:
>           type: array:
>           description: List of supported spec versions of this capability.
> 0 means experimental (non-production) without any guarantees about the
> stability of schema for request and response parameters.
>           items:
>             type: integer
>             format: int32
>
> In Nessie, we ensure backwards and forwards compatibility using a
> specialized test suite that runs the "in tree" client against older server
> versions and older client versions against the "in tree" server version. It
> works fine for us for a few years now - and it did help preventing
> compatibility issues.
>
>
> On 26.06.24 07:44, Péter Váry wrote:
>
> Hi everyone,
>
> A few considerations:
> - I think we should explicitly state which client/service interoperability
> we are aiming for. I expect that we want to support both old client -> new
> server, and new client -> old server communications.
> - I agree with Jack, that we should think about versions in advance - HMS
> tried to be backwards compatible for everything, and that made it hard to
> move forward / deprecate things.
> - Still we should try to keep the backwards incompatible changes minimal.
> (All clients should be able to ignore unknown incoming fields / New
> optional input parameter should drive new features / Try to avoid enums in
> responses where we expect changes (?))
> - OTOH, it could be important for clients to know which of the backwards
> compatible changes are implemented for the given server - so I would
> decouple the URI from the versioning. Maybe major version change should
> (could) change the URI, but backwards compatible changes should be served
> on the same URI, but could be identified by different minor versions.
>
> This is exciting stuff!
> Thanks for pushing this forward!
>
> Peter
>
>
> On Wed, Jun 26, 2024, 00:15 Jack Ye <yezhao...@gmail.com> wrote:
>
>> Hi everyone,
>>
>> I feel I do not see a good answer to why not just simply version each
>> API? When using tag, it means I have to offer capabilities per-tagged
>> group. However, I could for example just offer loadTable and nothing else
>> in a catalog, and that should still be Iceberg REST compliant. And I think
>> we need a versioning story anyway, there is no way around it.
>>
>> Here is the workflow in my mind with versioning:
>>
>> 1. Going forward, every time the REST catalog spec introduces any new API
>> endpoints or backwards incompatible changes to the existing APIs, the
>> version of the specific API is incremented. So suppose the PlanTable API is
>> added, this API will be at version v1. Suppose UpdateTable is updated with
>> a new update type, that API will be at version v2, but PlanTable will
>> remain at v1.
>>
>> 2. a catalog must implement getConfig. This API is the only one that is
>> required.
>>
>> 3. in getConfig, in the defaults map (it could be in some new metadata
>> structure, but since we want strong backwards compatibility guarantee,
>> reusing string maps seems to be the best way), server returns key-value
>> pairs of:
>> - key: operation:<operationName>
>> - value: version number
>>
>> 4. the client assumes that the map is ordered, and resolves API versions
>> sequentially. For example, suppose I have the following map:
>>
>> { "operation:planTable": "1", "operation:loadTable": "2" }
>>
>> Note that by "supporting", it means to return a response in a predictable
>> way that is compliant with the spec. It can also return 406
>> UnsupportedOperation as a way to support it.
>>
>> There is also a special version *, that means any version can work.
>>
>> 5. Backwards compatibility: suppose the client is at a higher version
>> than the server, then the client should always be able to understand the
>> server's full list of capabilities.
>>
>> 6. Forward compatibility: suppose the client is at a lower version than
>> the server, then the client should parse whatever operation it understands,
>> and use the highest version it could support to execute the operation.
>> Suppose the client only supports loadTable v1, then it will continue to hit
>> the GET v1/namespaces/{ns}/tables/{table} route, instead of GET
>> v2/namespaces/{ns}/tables/{table}. The v1 route could continue to support
>> the client, or it could throw 406 to indicate that this route is deprecated
>> and the client needs to upgrade.
>>
>> For initial backwards compatibility, I think not returning anything
>> should mean that all API that the client understands are having version *.
>>
>> What do people think of it, compared to the tag approach?
>>
>> Best,
>> Jack Ye
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Mon, Jun 24, 2024 at 1:42 PM Micah Kornfield <emkornfi...@gmail.com>
>> wrote:
>>
>>> I don't have strong opinions either way here, just thought it was worth
>>> raising some concerns over possible evolution here.  Some responses inline,
>>> but if capabilities seem to meet the requirement at hand, then it does
>>> potentially seem the simplest mechanism.
>>>
>>>
>>> I think we also want to avoid relyance on server specific published
>>>> OpenAPI as they may leak other options/parameters/etc.  This may lead to
>>>> confusion around what the canonical spec is and make clients incompatible
>>>> if they're generated off of a non-standard spec document.
>>>
>>>
>>> Yeah, I wasn't proposing necessarily using built in functionality but a
>>> pre-scrubbed document.  Since there is no reference service implementation
>>> for REST it seems like each implementor would need to describe the best way
>>> of scrubbing there description.
>>>
>>>
>>>
>>>> @Micah this sounds to me as if the client would then have to parse a
>>>> bunch of endpoints to figure out whether it's safe to e.g. call loading a
>>>> view or dropping a table on the given REST server. Rather than having a
>>>> dedicated endpoint we're just using the */config* endpoint to provide
>>>> information about what a server supports.
>>>
>>>
>>> I was not suggesting multiple endpoints here, simply different contents
>>> for */config *I agree in the short term this does add complexity on the
>>> clients. But given that the canonical REST API clients are being developed
>>> into the standard library, I'm not sure how much toil this would cause in
>>> general. This also does not necessarily need to called up-front but could
>>> be called to verify existence vs a permission issue after an error was
>>> received.
>>>
>>> What round-trips did you have in mind here?
>>>
>>>
>>> All good points though, but I'm not aware of a standard way to handle
>>>> this.
>>>
>>>
>>> IIUC, this sounds like a standard service description problem to me, the
>>> solution with capabilities appears to be one level abstraction on top of
>>> this.  Service discovery seems like it has been reimplemented a few
>>> different times depending on the technology [1][2][3]
>>>
>>>
>>> I think versioning adds another level of complexity, but might be
>>>> necessary since I expect these will evolve to some extent and may even
>>>> require hitting versioned urls.
>>>
>>>
>>> If there is no concrete proposal on versioning, I agree it probably pays
>>> to side step this.  The endpoint transitioning from list of strings to list
>>> of objects, would be an obvious sign to clients that they are out of date.
>>> I think serving a service description(s), despite its complexity, is likely
>>> the most principled way of versioning items appropriately, but this
>>> definitely requires more in depth thought/design.
>>>
>>>
>>> Thanks,
>>> Micah
>>>
>>> [1] https://en.wikipedia.org/wiki/Web_Services_Description_Language
>>> [2] https://en.wikipedia.org/wiki/Web_Application_Description_Language
>>> [3] https://developers.google.com/discovery/v1/reference/apis
>>>
>>>
>>>
>>>
>>> On Mon, Jun 24, 2024 at 12:42 PM Daniel Weeks <dwe...@apache.org> wrote:
>>>
>>>> Hey Micah,
>>>>
>>>> I think what we're trying to achieve is strike a balance between client
>>>> complexity and ability to support multiple server-side capabilities.  One
>>>> challenge we've run into is if a client performs an operation (e.g.
>>>> listViews), but receives a 403 code, it's not clear whether the client
>>>> doesn't have access or the server doesn't support an endpoint but isn't
>>>> sending a 404 for security reasons.  This is a simple way for the client to
>>>> understand what it should expect from the server.
>>>>
>>>> >  Another option would be just list all endpoints . . . and let
>>>> clients take appropriate actions
>>>> > This could be done by vending the OpenAPI spec the server supports at
>>>> its own endpoint. I think this avoids the future problem of having to
>>>> classify new endpoints into a specific capability.
>>>>
>>>> You're right that this would be the most complete way to handle this,
>>>> but it's really complicated and may require additional "handshake" calls
>>>> even for small interactions with the catalog service.  I think this puts a
>>>> lot of onus on the client, when what we're describing is a set of endpoints
>>>> that correspond to a capability.
>>>>
>>>> I think we also want to avoid relyance on server specific published
>>>> OpenAPI as they may leak other options/parameters/etc.  This may lead to
>>>> confusion around what the canonical spec is and make clients incompatible
>>>> if they're generated off of a non-standard spec document.
>>>>
>>>> All good points though, but I'm not aware of a standard way to handle
>>>> this.
>>>>
>>>> I think versioning adds another level of complexity, but might be
>>>> necessary since I expect these will evolve to some extent and may even
>>>> require hitting versioned urls.
>>>>
>>>> -Dan
>>>>
>>>>
>>>>
>>>>
>>>> On Mon, Jun 24, 2024 at 12:03 AM Eduard Tudenhöfner <
>>>> etudenhoef...@apache.org> wrote:
>>>>
>>>>> We had a separate discussion with Dan on the *oauth2* flag last week
>>>>> and came to the same conclusion that removing the *oauth2* capability
>>>>> is probably the best for now.
>>>>> This is mainly because we can't really act on the *oauth2* capability
>>>>> right now, because the */tokens* endpoint is called before we hit the
>>>>> */config* endpoint.
>>>>>
>>>>> > Another option would be just list all endpoints (and maybe even
>>>>> further which operations are supported) the server actually supports and
>>>>> let clients take appropriate actions (i.e. grouping could happen on the
>>>>> client side).  This could be done by vending the OpenAPI spec the server
>>>>> supports at its own endpoint. I think this avoids the future problem of
>>>>> having to classify new endpoints into a specific capability.
>>>>>
>>>>> @Micah this sounds to me as if the client would then have to parse a
>>>>> bunch of endpoints to figure out whether it's safe to e.g. call loading a
>>>>> view or dropping a table on the given REST server. Rather than having a
>>>>> dedicated endpoint we're just using the */config* endpoint to provide
>>>>> information about what a server supports.
>>>>>
>>>>> Thanks
>>>>> Eduard
>>>>>
>>>>> On Fri, Jun 21, 2024 at 8:27 PM Ryan Blue
>>>>> <b...@databricks.com.invalid> <b...@databricks.com.invalid> wrote:
>>>>>
>>>>>> Let's remove the oauth2 tag for now until we figure out how to move
>>>>>> forward there. That makes sense to me.
>>>>>>
>>>>>> On Fri, Jun 21, 2024 at 9:30 AM Dmitri Bourlatchkov
>>>>>> <dmitri.bourlatch...@dremio.com.invalid>
>>>>>> <dmitri.bourlatch...@dremio.com.invalid> wrote:
>>>>>>
>>>>>>> Hi Eduard,
>>>>>>>
>>>>>>> The capabilities PR looks good to me overall. I have a concern with
>>>>>>> the "oauth2" tag name though.
>>>>>>>
>>>>>>> I also commented [1] in GH but the comment appears to be closed by
>>>>>>> default :)
>>>>>>>
>>>>>>> I believe the term "oauth2" is confusing in this context with
>>>>>>> respect to RFC 6749 [2] as discussed in depth on another thread [3]
>>>>>>>
>>>>>>> The functionality behind the /tokens endpoint is quite specific to
>>>>>>> the Iceberg REST spec and as the other discussion highlights, there are
>>>>>>> concerns with respect to OAuth2 interoperability with other OAuth2 
>>>>>>> servers.
>>>>>>>
>>>>>>> What do you think about using a different tag name for it, for
>>>>>>> example "local-tokens" or "auth-tokens"?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Dmitri.
>>>>>>>
>>>>>>> [1]
>>>>>>> https://github.com/apache/iceberg/pull/9940/files/15c769a52b85ac4deff5659978c7ffa7802612b0#r1649173934
>>>>>>> [2] https://www.rfc-editor.org/rfc/rfc6749
>>>>>>> [3] https://lists.apache.org/thread/twk84xx7v0xy5q5tfd9x5torgr82vv50
>>>>>>>
>>>>>>> On Thu, Jun 20, 2024 at 7:28 AM Eduard Tudenhoefner <
>>>>>>> etudenhoef...@apache.org> wrote:
>>>>>>>
>>>>>>>> Hey everyone,
>>>>>>>>
>>>>>>>> I'd like to bring up the discussion around describing REST server
>>>>>>>> capabilities via the */config* endpoint.
>>>>>>>> There is PR #9940 <https://github.com/apache/iceberg/pull/9940> that
>>>>>>>> describes the OpenAPI spec changes.
>>>>>>>>
>>>>>>>> Mainly we'd like to have a *capabilities* field in the
>>>>>>>> *ConfigResponse* that allows servers to indicate to clients which
>>>>>>>> capabilities are being supported.
>>>>>>>>
>>>>>>>> So far we have the following capabilities:
>>>>>>>>
>>>>>>>>    - tables
>>>>>>>>    - views
>>>>>>>>    - remote-signing
>>>>>>>>    - vended-credentials
>>>>>>>>    - multi-table-commit
>>>>>>>>    - register-table
>>>>>>>>    - table-metrics
>>>>>>>>    - oauth2
>>>>>>>>
>>>>>>>>
>>>>>>>> The general idea behind a capability is that if e.g. a server
>>>>>>>> supports *views*, then that server must implement all endpoints
>>>>>>>> grouped under that capability.
>>>>>>>> It's worth noting that the */config* endpoint is currently being
>>>>>>>> implicit (meaning that every REST server would have to implement it).
>>>>>>>>
>>>>>>>> One discussion point that came up during review is how we want to
>>>>>>>> handle capabilities and backwards compatibility and what the default
>>>>>>>> capability would be, since older servers don't know anything about
>>>>>>>> *capabilities* (in such a case we could assume that the default
>>>>>>>> capabilities would be *oauth2* / *tables*).
>>>>>>>>
>>>>>>>> Are there any other capabilities that we'd like to include in the
>>>>>>>> list?
>>>>>>>>
>>>>>>>> Eduard
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Ryan Blue
>>>>>> Databricks
>>>>>>
>>>>> --
> Robert Stupp
> @snazy
>
>

Reply via email to