[pve-devel] [PATCH common v2] schema: only check for cycles during build

Thomas Lamprecht t.lamprecht at proxmox.com
Fri Nov 22 19:37:20 CET 2019


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.lamprecht at 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};
+    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





More information about the pve-devel mailing list