On 11/25/2013 03:27 PM, Jan Cholasta wrote:
On 8.11.2013 17:56, Petr Viktorin wrote:
Patch 198:
Also update ipaldap's find_entries docstring, it no longer uses IPA
defaults.
Done.
While you're touching this part of code, I had some other improvements
in mind -- you can consider them:
In find_entries,
attrs_list = [a.lower() for a in attrs_list]
to make sure 'memberindirect' is case insensitive
Done.
In get_memberof, construct `indirect` as a set, for Ο(1) remove().
^ ignore that, it's nuked in 201 \o/
Changing MEMBERS_ALL et.al. from numbers to descriptive strings, for
easier debugging.
^ these can be removed entirely in 201
Removed.
Patch 199: Looks great
Patch 200:
objtype, res_list, red_id, res_ctrls = result
Minor typo ----------^
Fixed.
This construction won't work as you'd expect in Python 2:
try:
(possibly raise interesting exception) (*)
except:
try:
(possibly raise exception to ignore) (**)
except:
pass
raise # (***)
The problem is that the exception in (**) overwrites the "current active
exception" raised in (*). In (***) the exception from the cleanup
will be
re-raised.
The solution is to store the wanted exception info, including the
traceback:
exc_type, exc_value, exc_traceback = sys.exc_info()
and then re-raise explicitly:
raise exc_type, exc_value, exc_traceback
Turns out LDAPError does not fly well with sys.exc_info() (all exception
data is lost, probably a python-ldap bug), so I used "except LDAPError,
e: ... raise e" instead.
Also, please log the ignored exception from cancelling the paged search.
Done.
Patch 201:
Great patch!
A nitpick, I'd rename _process_member{,of} to
_process_member{,of}indirect
Renamed.
Patch 202: Looks good
While we're on the subject: Each Plugin has an "api" attribute. It would
be nice if we started preferring `self.api` instead of the global
singleton wherever possible, even though they're currently always the
same.
+1, fixed.
Not fixed, but it's okay for now.
Updated patches attached.
Honza
Thanks! ACK, pushed to master: 4050e553c30d8c8d93c7045f871f8c1cef65aa71
--
Petr³
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
