On Wed, 2012-06-13 at 18:05 -0400, Rob Crittenden wrote: > Martin Kosek wrote: > > On Thu, 2012-06-07 at 22:38 -0400, Rob Crittenden wrote: > >> Martin Kosek wrote: > >>> When invalid data is passed, an unhandled decimal exception could > >>> be raised in Decimal number conversion. Handle the exception > >>> more gracefully and report proper ipalib.errors.ConversionError. > >>> > >>> https://fedorahosted.org/freeipa/ticket/2705 > >> > >> I'm being pedantic but I think the Decimal special values need to be > >> handled better. Using Infinity returns a rather odd message: > >> > >> $ ipa dnsrecord-add example.com > >> Record name: foo > >> Please choose a type of DNS resource record to be added > >> The most common types for this type of zone are: A, AAAA > >> > >> DNS resource record type: LOC > >> LOC Degrees Latitude: 90 > >> [LOC Minutes Latitude]: 59 > >> [LOC Seconds Latitude]: > >> 999999999999999999999999999999999999999999999999999999999999999999999 > >> >>> LOC Seconds Latitude: quantize result has too many digits for > >> current context > >> [LOC Seconds Latitude]: Infinity > >> >>> LOC Seconds Latitude: quantize with one INF > >> > >> And using NaN raises an unhandled exception: > >> > >> [LOC Seconds Latitude]: NaN > >> ipa: ERROR: InvalidOperation: comparison involving NaN > >> Traceback (most recent call last): > >> File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1263, in run > >> sys.exit(api.Backend.cli.run(argv)) > >> File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1049, in run > >> kw = self.argv_to_keyword_arguments(cmd, argv[1:]) > >> File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1035, in > >> argv_to_keyword_arguments > >> self.prompt_interactively(cmd, kw) > >> File "/home/rcrit/redhat/freeipa/ipalib/cli.py", line 1199, in > >> prompt_interactively > >> callback(kw) > >> File "/home/rcrit/redhat/freeipa/ipalib/plugins/dns.py", line 2147, > >> in interactive_prompt_callback > >> user_options = param.prompt_parts(self.Backend) > >> File "/home/rcrit/redhat/freeipa/ipalib/plugins/dns.py", line 768, in > >> prompt_parts > >> self.__get_part_param(backend, part, user_options, default) > >> File "/home/rcrit/redhat/freeipa/ipalib/plugins/dns.py", line 747, in > >> __get_part_param > >> output_kw[name] = part(raw) > >> File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 556, in > >> __call__ > >> self.validate(value, supplied=self.name in kw) > >> File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 879, in > >> validate > >> self._validate_scalar(value) > >> File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 893, in > >> _validate_scalar > >> error = rule(ugettext, value) > >> File "/home/rcrit/redhat/freeipa/ipalib/parameters.py", line 1244, in > >> _rule_minvalue > >> if value< self.minvalue: > >> File "/usr/lib64/python2.7/decimal.py", line 884, in __lt__ > >> ans = self._compare_check_nans(other, context) > >> File "/usr/lib64/python2.7/decimal.py", line 786, in > >> _compare_check_nans > >> self) > >> File "/usr/lib64/python2.7/decimal.py", line 3866, in _raise_error > >> raise error(explanation) > >> InvalidOperation: comparison involving NaN > >> ipa: ERROR: an internal error has occurred > >> > >> Otherwise it does what it should. > >> > >> rob > > > > Thanks for being pedantic, I found out that Decimal number validation > > and normalization needs more care, dnsrecord-add would also fail with > > values such as "1E4" or "-0". > > > > Attached patch improves Decimal number validation a lot and adds > > optional exponent normalization. I also added missing tests for all > > Decimal Parameter attributes. > > > > Martin > > Getting some lint errors. Ran out of time to investigate but its strange > because AFAICT these are members. > > ipalib/parameters.py:1266: [E1101, Decimal._enforce_numberclass] > Instance of 'Decimal' has no 'numberclass' member > ipalib/parameters.py:1271: [E1101, Decimal._enforce_numberclass] > Instance of 'Decimal' has no 'numberclass' member > ipalib/parameters.py:1288: [E1101, Decimal._remove_exponent] Instance of > 'Decimal' has no 'exponential' member >
I assume you run this lint: # ./make-lint ipalib/parameters.py This produces a lot of false positive lint errors... But if you run lint for the entire project, my patches will pass: # git apply /home/mkosek/freeipa-mkosek-275-2-decimal-parameter-conversion-and-normalization.patch # ./make-lint # That's why 275-2 includes a change for make-lint to not report these new attributes: diff --git a/make-lint b/make-lint index 7ecd59d7e8c5a644f812d4b8987866e7d06236b5..30c5e00c1f0606c75ff1f7fec675ff673a6b87a0 100755 --- a/make-lint +++ b/make-lint @@ -61,7 +61,8 @@ class IPATypeChecker(TypeChecker): 'csv', 'csv_separator', 'csv_skipspace'], 'ipalib.parameters.Bool': ['truths', 'falsehoods'], 'ipalib.parameters.Int': ['minvalue', 'maxvalue'], - 'ipalib.parameters.Decimal': ['minvalue', 'maxvalue', 'precision'], + 'ipalib.parameters.Decimal': ['minvalue', 'maxvalue', 'precision', + 'numberclass', 'exponential'], 'ipalib.parameters.Data': ['minlength', 'maxlength', 'length', 'pattern', 'pattern_errmsg'], 'ipalib.parameters.Enum': ['values'], Bottom line - I do not think there is a lint problem with my patch :-) Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel