From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id AB7C51FF163 for ; Thu, 21 Nov 2024 17:29:02 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 833B22304; Thu, 21 Nov 2024 17:29:09 +0100 (CET) Mime-Version: 1.0 Date: Thu, 21 Nov 2024 17:29:06 +0100 Message-Id: From: "Max Carrara" To: "Thomas Lamprecht" , "Proxmox VE development discussion" X-Mailer: aerc 0.18.2-0-ge037c095a049 References: <20240802132656.270077-1-m.carrara@proxmox.com> <20240802132656.270077-12-m.carrara@proxmox.com> In-Reply-To: X-SPAM-LEVEL: Spam detection results: 0 AWL 0.033 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [pbsclient.pm] Subject: Re: [pve-devel] [PATCH v1 pve-common 11/18] pbsclient: let `status` method return a hash instead of an array X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 > > --- > > 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