[Freeipa-devel] [PATCH] 0001 Raise ValidationError for incorrect subtree option
Martin Kosek
mkosek at redhat.com
Mon Jan 14 13:13:53 UTC 2013
On 01/04/2013 12:26 PM, Petr Viktorin wrote:
> On 01/03/2013 02:55 PM, Ana Krivokapic wrote:
>> On 01/03/2013 01:42 PM, Petr Viktorin wrote:
>>> On 01/03/2013 12:56 PM, Ana Krivokapic wrote:
>>>> Using incorrect input for --subtree option of ipa permission-add command
>>>> now raises a ValidationError.
>>>>
>>>> Previously, a ValueError was raised, which resulted in a user unfriendly
>>>> error message:
>>>> ipa: ERROR: an internal error has occurred
>>>>
>>>> I have added a try-except block to catch the ValueError and raise an
>>>> appropriate ValidationError.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/3233
>>>>
>>> ...
>>>
>>>> --- a/ipalib/plugins/aci.py
>>>> +++ b/ipalib/plugins/aci.py
>>>> @@ -341,7 +341,10 @@ def _aci_to_kw(ldap, a, test=False,
>>>> pkey_only=False):
>>>> else:
>>>> # See if the target is a group. If so we set the
>>>> # targetgroup attr, otherwise we consider it a subtree
>>>> - targetdn = DN(target.replace('ldap:///',''))
>>>> + try:
>>>> + targetdn = DN(target.replace('ldap:///',''))
>>>> + except ValueError as e:
>>>> + raise errors.ValidationError(name='subtree',
>>>> error=_(e.message))
>>>
>>> `_(e.message)` is useless. The message can only be translated if the
>>> string is grabbed by gettext, which uses static analysis. In other
>>> words, the argument to _() must be a literal string.
>>>
>>> You can do either `_("invalid DN")`, or if the error message is
>>> important, include it like this (e.message still won't be translated,
>>> but at least the users will get something in their language):
>>> _("invalid DN (%s)") % e.message
>>>
>>>
>> Fixed.
>>
>> Thanks, Petr.
>>
>
> ACK
>
Pushed to master, ipa-3-1.
Martin
More information about the Freeipa-devel
mailing list