On Wed, Feb 09, 2022 at 06:35:33PM +0100, Thomas Lamprecht wrote:
> On 27.01.22 11:55, Dominik Csapak wrote:
> > we will need some extra parameters here, and instead of hardcoding them,
> > have the option to set a list of arbitrary parameters
> >
> > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> > ---
> > src/PVE/PBSClient.pm | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
> > index 21dc363..dfb9f27 100644
> > --- a/src/PVE/PBSClient.pm
> > +++ b/src/PVE/PBSClient.pm
> > @@ -342,11 +342,15 @@ sub status {
> > };
> >
> > sub file_restore_list {
> > - my ($self, $snapshot, $filepath, $base64) = @_;
> > + my ($self, $snapshot, $filepath, $base64, $extraParams) = @_;
> > +
> > + my $params = [ $snapshot, $filepath, "--base64", $base64 ? 1 : 0 ];
> > + push @$params, @$extraParams;
> > +
> > return run_client_cmd(
> > $self,
> > "list",
> > - [ $snapshot, $filepath, "--base64", $base64 ? 1 : 0 ],
> > + $params,
> > 0,
> > "proxmox-file-restore",
> > );
>
> CC'ing Wolfgang, as IIRC he does not like passing "take anything" variables
> especially in such dynamic languages like perl.
>
> FWICT we only call file_restore_list once, and you set the options fixed
> there now, so why not just avoid the new method param and pass it directly
> fixed here?
Indeed, I'm not against having a hash there where we can specify
possible extra options, but not in the form of command line parameters
that are passed as-is. The question is rather how many more parameters
will be required in the future and will you just add a single `$timeout`
or a `%options` containing a `->{timeout}` field ;-)
And `--json-error` seems to be generally a good idea to be always set
for a CLI-tool-based perl api so it can just `die` with the right
errors, no? Wouldn't it make sense to add this directly to
`run_client_cmd`?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
