On 4/6/20 1:52 PM, Thomas Lamprecht wrote:
On 4/6/20 1:31 PM, Dominik Csapak wrote:we want the api options to be optional, but only as long as there are default values set in the realm config since they are all marked as optional (else they would be required in the api) this check did not work as intended instead, check if there exists a 'default' instead (which we then use) Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> --- PVE/API2/Domains.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm index 8ae1db0..b42d4f6 100644 --- a/PVE/API2/Domains.pm +++ b/PVE/API2/Domains.pm @@ -359,7 +359,7 @@ my $parse_sync_opts = sub { } elsif (!exists $res->{$opt}) { raise_param_exc({ "$opt" => 'Not passed as parameter and not defined in realm default sync options.' - }) if !$fmt->{optional}; + }) if !exists $fmt->{default}; $res->{$opt} = $fmt->{default} if exists $fmt->{default};The checks seems now redundant and possible incomplete, just because now all are always optional doesn't mean that it is the case in the future. We could do: raise_param_exc(...) if !$fmt->{optional} || !exists $fmt->{default}; $res->{$opt} = $fmt->{default};
but they always have to be optional, otherwise the code never reaches the point were it is not set but we enter the api call? we use the definitions both for the property string as well as the api parameters but yes, the !exists and exists a line after that makes little sense
Need to rethink this, as is it's not correct (thanks for noticing) but I think that I thought about something else here, damn brain..
_______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
