Re: [pve-devel] [RFC storage 10/23] plugin: introduce new_backup_provider() method

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

Am 26.07.24 um 14:02 schrieb Max Carrara:
> On Fri Jul 26, 2024 at 11:52 AM CEST, Fiona Ebner wrote:
>> Am 25.07.24 um 17:32 schrieb Max Carrara:
>>> On Thu Jul 25, 2024 at 3:11 PM CEST, Fiona Ebner wrote:
>>
>> I don't quite get your point about not needing to update the call sites.
>> If you change the structure of the passed-in hash you still need to do that.
> 
> Pardon me, I was a bit imprecise there. You're completely right that the
> passed hash has to be changed as well, of course.
> 
> What I meant in particular was that because the signature (most likely)
> won't change, we won't have to take special care to update the
> individual arguments that are passed to a method when e.g. a parameter
> is deprecated (or removed) or added.
> 
> For example, optional arguments are often just left out (because that's
> possible in Perl), so if we introduced another parameter ...
> 
> * after the optional one, we'd have to pass something like `0` or
>   `undef` for the optional one first before passing the new one:
> 
>     # before
>     $plugin->foo($first, $second);
> 
>     # after
>     $plugin->foo($first, $second, undef, $new); 
> 
> * before the optional one, we'd have to make sure the order of arguments
>   is correct:
> 
>     # before
>     $plugin->foo($first, $second, 1);
> 
>     # after
>     $plugin->foo($first, $second, $new, 1);
> 
> If we were to use a hash representing the parameters instead, the cases
> would look like this respectively:
> 
>     $plugin->foo({ first => $first, second => $second, new => $new });
> 
>     $plugin->foo({ first => $first, second => $second, optional => 1, new = $new });
> 
> More examples of that pattern would be `PVE::Tools::run_command` and
> `PVE::RADOS::mon_command`.
> 
> These changes can (and IMO should) still be guarded by an API age +
> version mechanism, it's just that the *surrounding maintenance work*
> becomes easier IMO, even if the initial cost for us and for implementors
> is a bit higher.
> 
> Of course, this is all a suggestion and I really don't want to seem like
> I'm telling you what to do here! If you decide not to have a parameter
> hash etc. then that's also completely fine by me, of course. :)
> 

Okay, I see what you mean. I agree that having a hash for optional
arguments is cleaner API-wise, but I do think fixed arguments (e.g. vmid
and drive ID for VM backup will always be present) should go directly
into the signature, not into a hash. I'll probably go with something
like what I wrote before about having mechanism-agnostic signatures, one
for VMs and one for containers.

> 
> Also, thanks for going through this discussion here with me - even if
> you choose to not incorporate my suggestions, I'm glad we have these
> little exchanges, because they periodically update my perspective(s) on
> Perl code as well :P
> 

No worries, your feedback is very welcome :)


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