Martin Kosek wrote:
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.

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
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]:
   >>>   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/", line 1263, in run
     File "/home/rcrit/redhat/freeipa/ipalib/", line 1049, in run
       kw = self.argv_to_keyword_arguments(cmd, argv[1:])
     File "/home/rcrit/redhat/freeipa/ipalib/", line 1035, in
       self.prompt_interactively(cmd, kw)
     File "/home/rcrit/redhat/freeipa/ipalib/", line 1199, in
     File "/home/rcrit/redhat/freeipa/ipalib/plugins/", line 2147,
in interactive_prompt_callback
       user_options = param.prompt_parts(self.Backend)
     File "/home/rcrit/redhat/freeipa/ipalib/plugins/", line 768, in
       self.__get_part_param(backend, part, user_options, default)
     File "/home/rcrit/redhat/freeipa/ipalib/plugins/", line 747, in
       output_kw[name] = part(raw)
     File "/home/rcrit/redhat/freeipa/ipalib/", line 556, in
       self.validate(value, in kw)
     File "/home/rcrit/redhat/freeipa/ipalib/", line 879, in
     File "/home/rcrit/redhat/freeipa/ipalib/", line 893, in
       error = rule(ugettext, value)
     File "/home/rcrit/redhat/freeipa/ipalib/", line 1244, in
       if value<   self.minvalue:
     File "/usr/lib64/python2.7/", line 884, in __lt__
       ans = self._compare_check_nans(other, context)
     File "/usr/lib64/python2.7/", line 786, in _compare_check_nans
     File "/usr/lib64/python2.7/", 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.


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.


Getting some lint errors. Ran out of time to investigate but its strange
because AFAICT these are members.

ipalib/ [E1101, Decimal._enforce_numberclass]
Instance of 'Decimal' has no 'numberclass' member
ipalib/ [E1101, Decimal._enforce_numberclass]
Instance of 'Decimal' has no 'numberclass' member
ipalib/ [E1101, Decimal._remove_exponent] Instance of
'Decimal' has no 'exponential' member

I assume you run this lint:

# ./make-lint ipalib/

This produces a lot of false positive lint errors... But if you run lint
for the entire project, my patches will pass:

# git
# ./make-lint

That's why 275-2 includes a change for make-lint to not report these new
diff --git a/make-lint b/make-lint
--- 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 :-)


This was output from './make-lint'. It blew up when I was trying to build the rpms.


Freeipa-devel mailing list

Reply via email to