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 DD3AC1FF17E for ; Thu, 2 Oct 2025 10:51:19 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 28EDBA564; Thu, 2 Oct 2025 10:51:26 +0200 (CEST) Message-ID: Date: Thu, 2 Oct 2025 10:50:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox VE development discussion , n.frey@proxmox.com References: <20250924115904.122696-1-n.frey@proxmox.com> <20250924115904.122696-3-n.frey@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20250924115904.122696-3-n.frey@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759395028812 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.026 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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. [apt.pm] Subject: Re: [pve-devel] [PATCH pve-manager v2 2/4] api: add APT versions return schema 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" Am 24.09.25 um 13:59 schrieb n.frey@proxmox.com: > From: Nicolas Frey > > listed only supported debian architectures [0] in enum for 'Arch'. > listed all APT package states in enum for 'CurrentState'. This is a bit specific and the sentence seems like it starts somewhere in the middle, maybe this can be lead up to with adding a little bit more context that avoids that one starts to wonder what a APT version return schema has to do with Debian A+rchs. > > [0] https://wiki.debian.org/SupportedArchitectures > > Signed-off-by: Nicolas Frey > --- > PVE/API2/APT.pm | 113 ++++++++++++++++++++++++++++-------------------- > 1 file changed, 67 insertions(+), 46 deletions(-) > > diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm > index 0d07cf38..885b85ae 100644 > --- a/PVE/API2/APT.pm > +++ b/PVE/API2/APT.pm > @@ -769,6 +771,25 @@ __PACKAGE__->register_method({ > }, > }); > > +$apt_package_return_props->{CurrentState} = { > + type => 'string', > + description => 'Current state of the package installed on the system.', > + # Possible CurrentState variants according to AptPkg::Cache > + enum => [qw(Installed NotInstalled UnPacked HalfConfigured HalfInstalled ConfigFiles)], > +}; > + > +$apt_package_return_props->{RunningKernel} = { > + type => 'string', > + description => "Kernel release, only for package 'proxmox-ve'.", > + optional => 1, > +}; > + > +$apt_package_return_props->{ManagerVersion} = { > + type => 'string', > + description => "Version of the currently running pve-manager API server.", > + optional => 1, > +}; > + Modifying a hash reference that is already used above is rather dangerous, as just some code line movement can introduce different behavior and wrong return types. You rather should create a new (cloned) variable, e.g.: my $versions_api_return_schema = { $apt_package_return_props->%*, CurrentState => { ..., }, ..., } As this is then only used once you can also do that inline below, as single-use intermediate variables are most often rather useless and just an extra indirection. While they sometimes the can indeed help bring clarity, here it would be pretty clear what's happening without that. > __PACKAGE__->register_method({ > name => 'versions', > path => 'versions', > @@ -788,7 +809,7 @@ __PACKAGE__->register_method({ > type => "array", > items => { > type => "object", > - properties => {}, > + properties => sort $apt_package_return_props, > }, > }, > code => sub { _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel