[Freeipa-devel] [PATCH] 0004 permission-add gives confusing error when adding ACI to generated tree
Martin Kosek
mkosek at redhat.com
Wed Oct 15 14:33:27 UTC 2014
On 10/15/2014 01:57 PM, thierry bordaz wrote:
> On 10/15/2014 01:26 PM, Martin Kosek wrote:
>> On 10/15/2014 01:08 PM, thierry bordaz wrote:
>>> https://fedorahosted.org/freeipa/ticket/4523
>> I see 2 issues with the patch:
>>
>> 1) Patch description should not contain "
>> Reviewed by:", this gets added later by a script (or human)
> ok
>>
>> 2) The exception handling clause should be as focused as possible, i.e. not
>> including whole command, but rather just the failing call, i.e.:
>>
>> def post_callback(self, ldap, dn, entry, *keys, **options):
>> try:
>> self.obj.add_aci(entry)
>> except Exception:
>>
>> You can use
>>
>> try:
>> ...
>> except errors.NotFound:
>> self.obj.handle_not_found(*keys)
>>
>> to raise the right error.
>>
>> Martin
> Currently the exception is handled on the failure of
> baseldap.LDAPCreate.execute(). Do you recommend to make the fix inside
> baseldap.LDAPCreate.execute rather than at the 'permission_add.execute' level ?
No, not there. I thought that the exception happens in
def post_callback(self, ldap, dn, entry, *keys, **options):
try:
self.obj.add_aci(entry)
except Exception:
...
> Also using handle_not_found looks good, but it reports something like:
>
> ipa permission-add user1 --right read --attrs cn --subtree
> 'cn=compat,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com'
> ipa: ERROR: user1: permission not found
>
>
> If the entry 'user1' exists, it is not clear what was not found.
> Displaying the dn of the entry would help to know that we are updating an entry
> into the 'compat' tree.
Ah, sorry, I think I mislead you with this advise. You probably could use the
same except clause as already used:
except errors.NotFound:
raise errors.ValidationError(
name='ipapermlocation',
error=_('Entry %s does not exist') % location)
Martin
More information about the Freeipa-devel
mailing list