On November 22, 2019 7:37 pm, Thomas Lamprecht wrote:
> Do not check for cycles or for self-validation if not in a build
> environment.
>
> The slight drawback is that we also avoid this cycle checks when we
> do (development) testing through the API and don't remeber to set the
> PROXMOX_FORCE_SCHEMA_VALIDATION environment variable.
>
> But honestyl, I never had a cycle in the API and got noticed by
> *this* check, as then to_json and the behavior were all the pointers
> needed to get this.
>
> Signed-off-by: Thomas Lamprecht <t.lampre...@proxmox.com>
> ---
>
> This time a bit more thought out (thanks Stoiko!)
>
> src/PVE/JSONSchema.pm | 38 +++++++++++++++++++++++++-------------
> 1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 51c3881..916f703 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -6,13 +6,21 @@ use Storable; # for dclone
> use Getopt::Long;
> use Encode::Locale;
> use Encode;
> -use Devel::Cycle -quiet; # todo: remove?
> use PVE::Tools qw(split_list $IPV6RE $IPV4RE);
> use PVE::Exception qw(raise);
> use HTTP::Status qw(:constants);
> use Net::IP qw(:PROC);
> use Data::Dumper;
>
> +my $do_build_checks;
> +BEGIN {
> + $do_build_checks = $ENV{DEB_BUILD_ARCH} ||
> $ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
couldn't we set 'PROXMOX_FORCE_SCHEMA_VALIDATION' in pve-doc-generator's
*api-verified targets and drop DEB_BUILD_ARCH here? seems a bit
arbitrary, and who knows what exports DEB_BUILD_ARCH into production
environments..
> + if ($do_build_checks) {
> + require Devel::Cycle;
> + import Devel::Cycle -quiet;
> + }
> +}
> +
> use base 'Exporter';
>
> our @EXPORT_OK = qw(
> @@ -1035,14 +1043,16 @@ sub validate {
> my $errors = {};
> $errmsg = "Parameter verification failed.\n" if !$errmsg;
>
> - # todo: cycle detection is only needed for debugging, I guess
> - # we can disable that in the final release
> - # todo: is there a better/faster way to detect cycles?
> - my $cycles = 0;
> - find_cycle($instance, sub { $cycles = 1 });
> - if ($cycles) {
> - add_error($errors, undef, "data structure contains recursive cycles");
> - } elsif ($schema) {
> + # only check when building a package or told to do so, this is costly
> + if ($do_build_checks) {
> + my $cycles = 0;
> + find_cycle($instance, sub { $cycles = 1 });
> + if ($cycles) {
> + add_error($errors, undef, "data structure contains recursive
> cycles");
> + }
> + }
> +
> + if ($schema) {
> check_prop($instance, $schema, '', $errors);
> }
>
> @@ -1400,10 +1410,12 @@ sub validate_method_info {
> validate_schema($info->{returns}) if $info->{returns};
> }
>
> -# run a self test on load
> -# make sure we can verify the default schema
> -validate_schema($default_schema_noref);
> -validate_schema($method_schema);
> +if ($do_build_checks) {
> + # run a self test on load when building
> + # make sure we can verify the default schema
> + validate_schema($default_schema_noref);
> + validate_schema($method_schema);
> +}
>
> # and now some utility methods (used by pve api)
> sub method_get_child_link {
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
