[pbs-devel] [PATCH v6 proxmox-backup 12/29] api/api-types: refactor api endpoint version, add api types

[Christian Ebner]

On 11/20/24 17:27, Thomas Lamprecht wrote:
> Am 06.11.24 um 12:57 schrieb Fabian Grünbichler:
>> @Thomas: since there's a few questions below that have long-term implications,
>> I'd appreciate feedback..
>>
>> Quoting Christian Ebner (2024-10-31 13:15:02)
>>> Add a dedicated api type for the `version` api endpoint and helper
>>> methods for supported feature comparison.
>>> This will be used to detect api incompatibility of older hosts, not
>>> supporting some features.
>>>
>>> Use the new api type to refactor the version endpoint and set it as
>>> return type.
>>>
>>
>> but, I am not sure if we even need this now, we could also just implement
>> helpers on ApiVersionInfo that give us the major, minor, release versions as
>> u64? especially if we do "does the server support XX" via explicit named
>> features, and don't even have a use case (yet) for accessing the version parts?
>>
>> the big question here is - do we want to expose this kind of thing?
>>
>> so far, we've used the approach of making things opt-in or backwards
>> compatible, or failing hard if a newer client tries to use a feature that is not
>> supported by an older server (e.g., if a client tries to use namespaces with a
>> server that doesn't support them, it will just error out on whichever request it
>> makes).
> 
> 
> For most new features this should be the way to go for now, as you write,
> we basically use it on every products' API already.
> 
> Such a feature negotiation makes IMO mostly sense if I can use that to
> fallback to some other protocol/enpoint/parameter set transparently while
> still honoring what the user told us to do here.

In this case we use the feature negotiation to expose and additional 
parameter to the snapshot/group delete endpoints, so that it behaves 
differently (no hard failure when protected snapshots are present, 
return delete stats). Without the feature exposed, the previous behavior 
is honored, which is still used for regular snapshot/group deletion.

> 
>>
>> there are two ways to handle explicit versioning between client and server:
>>
>> 1.) client retrieves the version, and has a list of "feature A is supported
>> since version X.Y.Z"
>>
>> 2.) client retrieves a list of supported features from the server (this patch
>> (series))
>>
>> variant 1 has the advantage that we don't have to keep an ever-growing list of
>> features around (or worry about "naming" and organizing them). variant 2 has the
>> advantage that the server can explicitly tell what it supports without needing
>> the client to adapt its version <-> feature mapping (i.e., if we or somebody else
>> backports a feature). it also has the advantage that there is no risk of the
>> version mapping being wrong (e.g., because there was unexpected delay in
>> applying a patch series, or somebody made a mistake in the contained version
>> number).
>>
>> variant 1 was what I actually had in mind when I originally proposed this, but I
>> do like variant 2 as well!
> 
> Not sure if we really want to backport features, at least if using the work in
> the more literal sense from a user POV. If, it would rather be for a security
> issue or grave bug fix, and it's hard to predict but IMO a bit unlikely that
> such things need features at a PBS-aware level that the API and its parameters
> cannot provide.
> 
> IMO it really depends on the specific use-case, and sure if we would add such
> feature negotiation endpoints and plumbing we might use it more, but mostly
> because it's there already, and probably not because it's strictly necessary.
> 
> But if we ignore the need then yes, feature lists might be a bit nicer, they
> decouple versioning and provide more semantic meaning on their own, that IME
> reduces error-potential to hold them wrong.

I would argue in favor of the feature list here, as this makes it:
- easier to see from the context what is needed
- independent of version bumps

> Anyway, I did not check this or v7 series out all to closely, so please bear
> with me, but from a 1 km view: I'd find it a bit unlikely that this series,
> which is adding new endpoints for new functionality needs such feature
> detection or negotiation, users simply need to upgrade in any way.
> So, for the sake of simplicity, which often has a big value for long-term
> maintenance, let's avoid this if not really required.