Re: [Kimchi-devel] [PATCHv3 3/8] Add LDAP authentication

On 13-11-2014 05:44, Royce Lv wrote: >> I think the code would look better without the external try/except >> block. > Do you mean get them handled all by kimchi exception? > I've considered that, but like: > connect.search_s() > connect.bind_s() > connect.unbind_s() > all throw ldap.LDAPError, they also throw exception like: > ldap.INVALD_CREDENTIALS. > so if delete the external try/except, I need to add 3 excepts to each > operation. > You can see I do this because ldap lib does not handle all exception > with ldap.LDAPError. > Wouldn't something like this work? try: result = connect.search_s( ldap_search_base, ldap.SCOPE_SUBTREE, ldap_search_filter) if len(result) == 0: entity = ldap_search_filter % {'username': username} raise ldap.LDAPError("Invalid ldap entity:%s" % entity) connect.bind_s(result[0][0], password) connect.unbind_s() return True except ldap.INVALID_CREDENTIALS: # invalid user password arg = {"username": username, "code": "invalid user/passwd"} raise OperationFailed("KCHAUTH0001E", arg) except ldap.NO_SUCH_OBJECT: # ldap search base specified wrongly. arg = {"username": username, "code": "invalid ldap search base %s" % ldap_search_base)} raise OperationFailed("KCHAUTH0001E", arg) except ldap.LDAPError, e: arg = {"username": username, "code": e.message} raise OperationFailed("KCHAUTH0001E", arg) Maybe you could even have different messages ID instead of passing "code" like that. Keep in mind that those "codes" won't be translated... But that's just code preference, I just think this looks better than using a nested block.