Re: [pve-devel] [PATCH manager v13 08/12] api: add resource map api endpoints for directories

[Fiona Ebner <f.ebner@proxmox.com>]

Am 22.01.25 um 11:08 schrieb Markus Frank:
> Signed-off-by: Markus Frank <m.frank@proxmox.com>

Some minor nits, but otherwise:

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

> diff --git a/PVE/API2/Cluster/Mapping/Dir.pm b/PVE/API2/Cluster/Mapping/Dir.pm
> new file mode 100644
> index 00000000..5218241f
> --- /dev/null
> +++ b/PVE/API2/Cluster/Mapping/Dir.pm
> @@ -0,0 +1,307 @@
> +package PVE::API2::Cluster::Mapping::Dir;
> +
> +use strict;
> +use warnings;
> +
> +use Storable qw(dclone);
> +
> +use PVE::INotify;
> +use PVE::JSONSchema qw(get_standard_option parse_property_string);

Nit: parse_property_string is not used.

> +use PVE::Mapping::Dir ();
> +use PVE::RPCEnvironment;

Missing
use PVE::SectionConfig;
because of PVE::SectionConfig::delete_from_config() below

> +use PVE::Tools qw(extract_param);
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    # only proxy if we give the 'check-node' parameter
> +    proxyto_callback => sub {
> +	my ($rpcenv, $proxyto, $param) = @_;
> +	return $param->{'check-node'} // 'localhost';
> +    },
> +    description => "List directory mapping",
> +    permissions => {
> +	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or".
> +	    " 'Mapping.Audit' permissions on '/mapping/dir/<id>'.",

I know these are copied, but while we're at it:

Style nit: dot should be on this line
https://pve.proxmox.com/wiki/Perl_Style_Guide#Wrapping_Strings

> +	user => 'all',
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    'check-node' => get_standard_option('pve-node', {
> +		description => "If given, checks the configurations on the given node for ".
> +		    "correctness, and adds relevant diagnostics for the directory to the response.",

Style nit: dot and space should be on this line


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