Re: [pve-devel] [PATCH storage] volume import: assume target API version is at least 9

[Fiona Ebner <f.ebner@proxmox.com>]

Am 04.07.24 um 19:45 schrieb Thomas Lamprecht:
> Am 04/07/2024 um 14:11 schrieb Fiona Ebner:
>> There is no apiinfo call required anymore. No code is the cleanest kind
> 
> Yeah, by the assumption you self choose to use and that I question, so
> not really a useful argument.
> 
> In practice, users can upgrade a from one major release to the next one,
> nothing is really blocking them w.r.t apt, that cannot be said for jumping
> releases, so one should definitively have different levels of standard for
> "any X to any X + 1" compared to "X to X + >= 2".
> 
> What we recommend users (run latest 7 before upgrade) is not necessarily
> the exact same boundary of what we should hedge against to reduce negative
> feelings of users and resulting work/soothing our support has to do as
> a result; I'd think of it more like the minimum and recommended system
> requirements.
> 

Okay, I understand.

>> of code IMHO.
> 
> Less code can be nicer as long as all features and edge cases are still
> handled properly and also still easy to read, not code golf minimalism.
> 
> Or, to exaggerate for the points' sake, should I just delete all our
> (user) error code handling and tell any users complaining that they just
> need to do it right the next time? While that would result in quite
> a few lines less, it definitively won't help our popularity.
> 

Of course I'm not arguing for such a thing. Being accused of arguing for
that hurts :(, especially when coming from somebody I highly respect. I
honestly did not know that we consider having a node with PVE 7.0 while
upgrading to PVE 8 supported.

>>> Besides that: no, I definitively place UX over some abstract "code cleanliness"
>>> criteria – if, it can be fair to set the barrier such that one should achieve
>>> both, but putting UX below code tidiness is IMO just not acceptable.
>>
>> I do put UX for users running ancient versions below code cleanliness,
>> but not UX for current versions of course.
> 
> PVE 7 is still supported, so definitively not ancient! We normally put
> in quite a bit of work to ensure that systems talking with other
> incompatible versions get a good error, why bother adding versioning
> else in the first place? And while there's definitively areas that can
> still be improved, that's no argument for going backwards again.
> 

Again, it's not all of PVE 7, just an early subset of it that's about
three years without updates. If it weren't that, I wouldn't have it
considered safe to remove the call. But okay, I'm fine with being more
strict.

> I mean this specific case here might be relatively harmless in effect,
> but if you really apply this philosophy to all development here then I
> wish you good luck into explaining angry users and our support agents
> that had to answer them, why it was good for them to remove some simple
> check for code cleanliness, because that should not be relevant if
> the users did everything 100% correct. We already need to justify
> us too much about not being hacky as a FLOSS solution, after having
> to use the shell occasionally any missing/incomplete error handling is
> the next big reason for people to complain.
> And sure, most alternative (proprietary) solutions are far from being
> better, and while it's annoying that some people are hypocrites here,
> I do not have anything against being held to a high(er) standard,
> as one can really stick out with good UX, which always consists of tons
> of little things.
> 
> Anyhow, as mentioned, this might not fall back on us here, but I hope
> I could convince to not use that philosophy unconditionally, and I
> wished for some more background explanation on this in the commit message.
> 
> I just cannot think that there could have any negative effect, be it in
> using nor maintaining that code for neither users nor developers, if we
> just fixed the actual (error handling) behavior of querying the API
> version instead, and possibly dropped the real old version checks, i.e.
> those not just a single major version apart, in a separate patch.

Let's just revert it then and drop the eval and the fallback. Fine by
me. Or actually, a fresh install of PVE 7.0 comes with
libpve-storage-perl = 7.0-7 (respectively 7.0-10 for the second release
of the ISO). The API version was bumped to 9 in 7.0-4, so actually the
subset of PVE 7 that's affected is empty.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel