On 10/15/2014 01:26 PM, Martin Kosek wrote:
On 10/15/2014 01:08 PM, thierry bordaz wrote:
I see 2 issues with the patch:
1) Patch description should not contain "
Reviewed by:", this gets added later by a script (or human)
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'
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):
You can use
to raise the right error.
Also using handle_not_found looks good, but it reports something like:
ipa permission-add user1 --right read --attrs cn --subtree
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.
Freeipa-devel mailing list