Re: [pve-devel] [PATCH v1 pve-common 11/18] pbsclient: let `status` method return a hash instead of an array

["Max Carrara" <m.carrara@proxmox.com>]

On Mon Nov 11, 2024 at 8:17 PM CET, Thomas Lamprecht wrote:
> Am 02.08.24 um 15:26 schrieb Max Carrara:
> > Instead of returning an (arguably awkward) array with four elements,
>
> can be fine, but FWIW, this is how our storage plugin system expects the
> status, but currently that calls run_client_cmd itself, i.e. is basically a
> copy of the original code here.

Ah, thanks for pointing that out, I hadn't considered this, as the
`status` sub isn't in use currently.

Wouldn't it make sense to design the API of PVE::PBSClient independent
of other components, though? Of course it should be able to do what
other system components need it to do, but in this case, returning an
array is much less extendable (and IMO readable) than returning a hash.

To elaborate on my reasoning here: Since I had assumed that this is a
more general purpose PBS client lib, I didn't want its return types,
signatures, etc. to be too restrictively designed.

>
> > where each element has a special meaning, return the hash that is
> > constructed from the `proxmox-backup-client status` command's JSON
> > output.
> > 
> > The documentation is updated accordingly.
> > 
> > This method isn't used anywhere in our code base at the moment, so I
> > assume it is safe to change it. Checked with ripgrep.
> > 
> > Signed-off-by: Max Carrara <m.carrara@proxmox.com>
> > ---
> >  src/PVE/PBSClient.pm | 32 ++++++++++----------------------
> >  1 file changed, 10 insertions(+), 22 deletions(-)
> > 



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