[Freeipa-devel] [PATCH] 0001 Raise ValidationError for incorrect subtree option

[Petr Viktorin]

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

-- 
Petr³