JR Aquino wrote:
On Apr 12, 2011, at 10:55 AM, Rob Crittenden wrote:
JR Aquino wrote:
On Apr 7, 2011, at 7:08 PM, JR Aquino wrote:
On Apr 7, 2011, at 4:04 PM, JR Aquino wrote:
On Apr 7, 2011, at 3:42 PM, JR Aquino wrote:
On Mar 31, 2011, at 2:16 PM, JR Aquino wrote:
On Mar 31, 2011, at 1:48 PM, Rob Crittenden wrote:
JR Aquino wrote:
The following patch Removes around 20 lines of code and provides a substantial
increase in performance for FreeIPA member/memberof verification searches.
The current code base blindly searches static containers for the possible
presence of members.
This patch provides a method for dynamically identifying the specific objects
to verify memberships for.
The attached patch addresses ticket:
https://fedorahosted.org/freeipa/ticket/1139
<Without patch>
ipa hostgroup-find
...
-----------------------------
Number of entries returned 52
-----------------------------
real 0m20.054s
user 0m0.934s
sys 0m0.050s
<With Patch>
ipa find-hostgroup
...
-----------------------------
Number of entries returned 52
-----------------------------
real 0m15.064s
user 0m0.945s
sys 0m0.057s
------------------------------
Number of entries returned 100
------------------------------
real 0m16.471s
user 0m0.814s
sys 0m0.040s
<Without Patch>
ipa host-find
...
------------------------------
Number of entries returned 100
------------------------------
real 0m41.277s
user 0m0.806s
sys 0m0.060s
<With Patch>
ipa host-find
...
------------------------------
Number of entries returned 100
------------------------------
real 0m16.385s
user 0m0.814s
sys 0m0.053s
There is a typo in the first block, memeber.
Wouldn't it be clearer to do a negative test to continue:
if not 'member' in r[1]:
continue
rob
You're right!
Corrected patch attached.
Self Nack
After cli and webui testing, it turned out there was a previous try / except
block that was reseting the results value back to []
Corrected and reattaching new patch.
Testing cli and webui checks out correctly. Speed AND accuracy are now
addressed.
It was also discovered during the course of testing that this patch addresses
one of the causes for the bug thrown in:
https://fedorahosted.org/freeipa/ticket/1133
-JR
NACK
Looks like there may still need to be work with the indirect / direct functions.
Will revisit next week.
Ok I finally think I've got it.
My for loop was in my try / except block. It has now been corrected.
I've tested the searches for: users, groups, sudocmds, sudcmdgroups, sudorules,
hosts, hostgroups, hbacrules, hbacsv, hbsvcgroups, and all return as expected.
Please make sure that they return for you as well.
Please let me know if there is anything else I have missed.
Final Patch attached due to relationship with ticket 1133:
Added Comments regarding Ticket 1133 / calculating indirect:
+ # If there is an exception here, it is due to a failure in referential
integrity.
+ # All members should have corresponding memberOf entries.
Retested against all xmlrpc tests and passed.
Seems to work as advertised, I just have a couple of requests:
- Some of the comments are really long, can you limit to ~75 chars per line?
- In this code block:
for r in results:
direct.append(r[0])
try:
indirect.remove(r[0].lower())
except ValueError:
pass
We should log if a ValueError is thrown, this would mean something is really
wrong. Can you import logging in the file and replace pass with something like:
logging.info('Failed to remove indirect entry %s from %s' % r[0], entry_dn)
I wonder if we should raise the ValueError too. This means that something went
very wrong.
Yes I agree that we should raise the error.
Here is the patch with the requested changes:
* Fixed width to be PEP8 compliant
* import logging
* Added logging line in exception
* Raise ValueError if we blow up during indirect removal.
Fixes 1-3 look good. I think for the exception you want:
except ValueError, e:
logging ....
raise e
A ValueError won't get returned over XML-RPC but the full backtrace will
be logged on the server side. The end-user will get a 903 (Internal
error raised).
rob
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
