[pve-devel] [PATCH pve-manager v2 2/4] api: add APT versions return schema
Nicolas Frey
n.frey at proxmox.com
Thu Oct 2 11:25:47 CEST 2025
On 10/2/25 10:50 AM, Thomas Lamprecht wrote:
> Am 24.09.25 um 13:59 schrieb n.frey at proxmox.com:
>> From: Nicolas Frey <n.frey at proxmox.com>
>>
>> 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.
>
Will add context in v3, thanks!
>>
>> [0] https://wiki.debian.org/SupportedArchitectures
>>
>> Signed-off-by: Nicolas Frey <n.frey at proxmox.com>
>> ---
>> 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.
Thanks for the feedback! I had a feeling modifying the hash reference might
not be the best approach (still getting used to Perl).
Will address in v3.
>
>> __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 {
>
More information about the pve-devel
mailing list