Martin Kosek wrote:
On Mon, 2012-03-19 at 14:43 -0400, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/19/2012 03:02 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/16/2012 10:23 PM, Rob Crittenden wrote:
When using *attr we should return the param.name of in the exception
and
when using a cli option we should return param.cli_name. This didn't
work consistently in the framework.
This is a bit of a kludge, catching exceptions and re-raising them, but
it is a less invasive way of doing it.
I added some examples of things to test in the ticket.
rob
(name, error) = err.strerror.split(':')
raise errors.ConversionError(name=attr, error=error)
AFAIU this will break when the error message is translated. Why not just
use err.kw['error']?
Because the attribute name needs to get pulled out of it.
rob
No, you're ignoring the attribute name.
Anyway, even the English error messages are 'invalid %(name)r:
%(error)s', so "name" part starts with 'invalid ', and the "error" part
starts with an extra space.
The error gets re-raised so there is no dup "invalid". I'll grant you
the extra space though.
What this does is catch something like 'invalid maxfail: something bad',
split out the attribute/error and re-raise use the attribute name we want.
I think this is what Petr wanted to point out - you can access the
attributes passed to our PublicErrors directly, i.e. you don't have to
parse it from its string representation:
from ipalib import errors
x = errors.ValidationError(name='foo', error=u'Invalid value!')
print str(x)
invalid 'foo': Invalid value!
x.name
'foo'
x.error
u'Invalid value!'
Martin
Of course, can't believe I missed that.
Rebased against ipa-2-2.
rob
>From 8dd21bb59c1224bd19d1b4930ecc2a1ede60ae7a Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 16 Mar 2012 13:30:59 -0400
Subject: [PATCH] Use a consistent parameter name in errors, defaulting to
cli_name.
For general command-line errors we want to use the cli_name on output.
The exception is when using *attr, we want to return that attribute name
in the exception.
https://fedorahosted.org/freeipa/ticket/1418
---
ipalib/parameters.py | 35 +++++++++++++++++++++--------------
ipalib/plugins/baseldap.py | 5 +++--
2 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 48155daf9bc69f6b50f390737b8b69a2c84e12a2..94f11d912c010fb644d9683d7630a2d753f06371 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -563,6 +563,18 @@ class Param(ReadOnly):
self.validate(value, supplied=self.name in kw)
return value
+ def get_param_name(self):
+ """
+ Return the right name of an attribute depending on usage.
+
+ Normally errors should use cli_name, our "friendly" name. When
+ using the API directly or *attr return the real name.
+ """
+ name = self.cli_name
+ if not name:
+ name = self.name
+ return name
+
def kw(self):
"""
Iterate through ``(key,value)`` for all kwargs passed to constructor.
@@ -861,11 +873,8 @@ class Param(ReadOnly):
for rule in self.all_rules:
error = rule(ugettext, value)
if error is not None:
- name = self.cli_name
- if not name:
- name = self.name
raise ValidationError(
- name=name,
+ name=self.get_param_name(),
value=value,
index=index,
error=error,
@@ -1175,7 +1184,7 @@ class Int(Number):
return int(value)
except ValueError:
pass
- raise ConversionError(name=self.name, index=index,
+ raise ConversionError(name=self.get_param_name(), index=index,
error=ugettext(self.type_error),
)
@@ -1218,11 +1227,8 @@ class Int(Number):
for rule in self.all_rules:
error = rule(ugettext, value)
if error is not None:
- name = self.cli_name
- if not name:
- name = self.name
raise ValidationError(
- name=name,
+ name=self.get_param_name(),
value=value,
index=index,
error=error,
@@ -1309,7 +1315,7 @@ class Decimal(Number):
try:
value = decimal.Decimal(value)
except Exception, e:
- raise ConversionError(name=self.name, index=index,
+ raise ConversionError(name=self.get_param_name(), index=index,
error=unicode(e))
if isinstance(value, decimal.Decimal):
@@ -1449,7 +1455,7 @@ class Bytes(Data):
try:
value = base64.b64decode(value)
except TypeError:
- raise ConversionError(name=self.name, index=index, error=self.type_error)
+ raise ConversionError(name=self.get_param_name(), index=index, error=self.type_error)
return super(Bytes, self)._convert_scalar(value, index)
@@ -1548,7 +1554,8 @@ class IA5Str(Str):
if isinstance(value, basestring):
for i in xrange(len(value)):
if ord(value[i]) > 127:
- raise ConversionError(name=self.name, index=index,
+ raise ConversionError(name=self.get_param_name(),
+ index=index,
error=_('The character \'%(char)r\' is not allowed.') %
dict(char=value[i],)
)
@@ -1832,10 +1839,10 @@ class AccessTime(Str):
try:
self._check(value)
except ValueError, e:
- raise ValidationError(name=self.cli_name, error=e.args[0])
+ raise ValidationError(name=self.get_param_name(), error=e.args[0])
except IndexError:
raise ValidationError(
- name=self.cli_name, error='incomplete time value'
+ name=self.get_param_name(), error='incomplete time value'
)
return None
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 92540d8ac38a9fe033327d98c949469cb4745a97..6b629dc248b8a08137adcf854c94033271d86b7b 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -780,8 +780,9 @@ last, after all sets and adds."""),
try:
value = self.params[attr](value)
except errors.ValidationError, err:
- (name, error) = str(err.strerror).split(':')
- raise errors.ValidationError(name=attr, error=error)
+ raise errors.ValidationError(name=attr, error=err.error)
+ except errors.ConversionError, err:
+ raise errors.ValidationError(name=attr, error=err.error)
if self.api.env.in_server:
value = self.params[attr].encode(value)
if append and attr in newdict:
--
1.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
