On 11/25/19 11:21 AM, Fabian Grünbichler wrote:
> 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..
Not really realistic on a PVE machine, especially if not DEB, and in
the "worst" case you're as good as we're now - i.e., do some additional
checks and get taught to not system wide or (for root) export
DEB_BUILD_ARCH ;)
I don't want another versioned dependency bump over the whole three,
just to ensure that the cycle checks and initial self-tests are done..
This was re-using something existing.
>
>> + 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
