On 5/26/20 3:42 PM, Moayad Almalat wrote:
> Signed-off-by: Moayad Almalat <m.alma...@proxmox.com>
> ---
please add a changelog here, e.g.
changes from v2 -> v3:
* set returns property for API schema
* ...
FYI: you can add a "v2" easily by using the -v2 option in `git format-patch`
or `git send-email`.
> PVE/API2/Subscription.pm | 24 ++++++++++++++++++++++++
> PVE/CLI/pvesubscription.pm | 1 +
> www/manager6/node/Subscription.js | 9 +++++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/PVE/API2/Subscription.pm b/PVE/API2/Subscription.pm
> index 6657c00d..b0597cc2 100644
> --- a/PVE/API2/Subscription.pm
> +++ b/PVE/API2/Subscription.pm
> @@ -245,4 +245,28 @@ __PACKAGE__->register_method ({
> return undef;
> }});
>
> +__PACKAGE__->register_method ({
> + name => 'delete',
> + path => '',
> + method => 'DELETE',
> + permissions => {
> + check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
> + },
> + description => "Delete subscription key.",
> + proxyto => 'node',
> + protected => 1,
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + },
> + },
> + returns => { type => 'null'},
> + code => sub {
> + my $subscription_file = '/etc/subscription';
> + die "Subscription file doesn't exist - $!\n" if ! -e $subscription_file;
> + unlink $subscription_file if -e $subscription_file;
Twice making the same check makes not much sense, or?
I made two simple suggestions and you go a complete other direction, why?
So please, no die on file doesn't exists and check the return value of
unlink for failure. In other words, these two lines:
return if ! -e $subscription_file; # nothing to delete
unlink($subscription_file) or die "cannot delete subscription key: $!";
The rest of the patch looks now much better, thanks for that!
> + return undef;
> + }});
> +
> 1;
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
