Re: [Freeipa-devel] [PATCH] 899 more context with attribute in error message

2011-12-09 Thread Rob Crittenden

Martin Kosek wrote:

On Fri, 2011-10-21 at 15:28 -0400, Rob Crittenden wrote:

Depending on the context and how you are using input (-- options or
set/addattr) you might get a different attribute name in the error
message. This patch tries to clear that up a bit.

See the ticket for some test cases.

rob


I found few issues with this patch:

1) This patch cannot be applied. First chunk (frontend.py) was already
pushed in other patch

2) Actually, this patch did not work for me:

# ipa pwpolicy-mod --setattr=krbpwdmaxfailure=-1
ipa: ERROR: invalid 'krbpwdmaxfailure':  must be at least 0

cli_name still did not show up.

I also see that you did the change for Int onlu. I guess we should fix
all parameters, for example StrEnum, etc.:

# ipa sudorule-add foo --usercat=bar
ipa: ERROR: invalid 'usercat': must be one of (u'all',)
# ipa sudorule-add foo --setattr=usercategory=bar
ipa: ERROR: invalid 'usercategory':  must be one of (u'all',)

Martin



This must have gotten fixed by some other patch. This is the behavior we 
want. When using a named option we should return that in the error 
message. When using an attribute name we should return that.


I think this ticket can be closed.

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 899 more context with attribute in error message

2011-10-21 Thread Rob Crittenden
Depending on the context and how you are using input (-- options or 
set/addattr) you might get a different attribute name in the error 
message. This patch tries to clear that up a bit.


See the ticket for some test cases.

rob
From 3874e15bc26a6bca126838af64462214d0bddce8 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 21 Oct 2011 15:21:45 -0400
Subject: [PATCH] Be more consistent when returning the attribute in error
 messages.

Use whatever context when have (attr vs cli_name) when returning
error messages. When --set/addattr are used try to return that value,
otherwise return cli_name if we have it.

https://fedorahosted.org/freeipa/ticket/1418
---
 ipalib/frontend.py   |8 ++--
 ipalib/parameters.py |5 -
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 61e7f49..9ddef63 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -30,7 +30,7 @@ from util import make_repr
 from output import Output, Entry, ListOfEntries
 from text import _, ngettext
 
-from errors import ZeroArgumentError, MaxArgumentError, OverlapError, RequiresRoot, VersionError, RequirementError
+from errors import ZeroArgumentError, MaxArgumentError, OverlapError, RequiresRoot, VersionError, RequirementError, ValidationError
 from errors import InvocationError
 from constants import TYPE_ERROR
 from ipapython.version import API_VERSION
@@ -551,7 +551,11 @@ class Command(HasParam):
 # None means delete this attribute
 value = None
 if attr in self.params:
-value = self.params[attr](value)
+try:
+value = self.params[attr](value)
+except ValidationError, err:
+(name, error) = err.strerror.split(':')
+raise ValidationError(name=attr, error=error)
 if append and attr in newdict:
 if type(value) in (tuple,):
 newdict[attr] += list(value)
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index f9e171b..2d5367b 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -1043,7 +1043,10 @@ class Int(Number):
 return int(value)
 except ValueError:
 pass
-raise ConversionError(name=self.name, index=index,
+name = self.cli_name
+if not name:
+name = self.name
+raise ConversionError(name=name, index=index,
 error=ugettext(self.type_error),
 )
 
-- 
1.7.6

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel