public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH common] get_options: allow optional arguments "arg_params" if no ambiguity
Date: Wed, 26 Aug 2020 21:27:10 +0200	[thread overview]
Message-ID: <20200826192710.2131502-1-t.lamprecht@proxmox.com> (raw)

If we run out of passed arguments from the user but still had defined
"arg_params" (those params which went after the command in fixed
order without option -- dashes) we always errored out with "not
enough arguments". But, there are situations where the remaining
arg_params are all marked as optional in the schema, so we do not
need to error out in that case.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

A prime (future) use case is "pvesm prune-backups". Currently the
usage is:
> pvesm prune-backups storeid --prune-backups keep-last=1,keep-...

Because the "prune-backups" keep retention property is optional as it
can fallback to the one defined in the storage configuration.
With this patch we can make it an argument and allow the following
two usages:

1. As above, but avoiding the extra ugly --prune-backups
> pvesm prune-backups storeid keep-last=1,keep-...

2. Fallback to storage config:
> pvesm prune-backups storeid

 src/PVE/JSONSchema.pm | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index c5002d8..2ceb1bd 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1613,7 +1613,8 @@ sub get_options {
 	    $opts->{$list_param} = $args;
 	    $args = [];
 	} elsif (ref($arg_param)) {
-	    foreach my $arg_name (@$arg_param) {
+	    for (my $i = 0; $i < scalar(@$arg_param); $i++) {
+		my $arg_name = $arg_param->[$i];
 		if ($opts->{'extra-args'}) {
 		    raise("internal error: extra-args must be the last argument\n", code => HTTP_BAD_REQUEST);
 		}
@@ -1622,7 +1623,15 @@ sub get_options {
 		    $args = [];
 		    next;
 		}
-		raise("not enough arguments\n", code => HTTP_BAD_REQUEST) if !@$args;
+		if (!@$args) {
+		    # check if all left-over arg_param are optional, else we
+		    # must die as the mapping is then ambigious
+		    for (my $j = $i; $j < scalar(@$arg_param); $j++) {
+			my $prop = $arg_param->[$j];
+			raise("not enough arguments\n", code => HTTP_BAD_REQUEST)
+			    if !$schema->{properties}->{$prop}->{optional};
+		    }
+		}
 		$opts->{$arg_name} = shift @$args;
 	    }
 	    raise("too many arguments\n", code => HTTP_BAD_REQUEST) if @$args;
-- 
2.20.1





             reply	other threads:[~2020-08-26 19:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-26 19:27 Thomas Lamprecht [this message]
2020-08-27  4:22 ` Dietmar Maurer
2020-08-27  6:39   ` Thomas Lamprecht
2020-08-27  7:54     ` Fabian Ebner
2020-08-27  8:54 ` [pve-devel] applied: " Thomas Lamprecht
2020-08-31 10:25   ` Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200826192710.2131502-1-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal