<div dir="ltr"><div><div>Makes sense. I also changed the doc string to reflect -1 as well. Updated patch attached. </div>Thanks, </div>Gabe <div><div><div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Sep 10, 2015 at 1:41 AM, Jan Cholasta <<a href="mailto:jcholast@redhat.com" target="_blank">jcholast@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 4.9.2015 14:43, Gabe Alford wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Bump for review. On Wed, Aug 12, 2015 at 9:32 AM, Gabe Alford <<a href="mailto:redhatrises@gmail.com" target="_blank">redhatrises@gmail.com</a> <mailto:<a href="mailto:redhatrises@gmail.com" target="_blank">redhatrises@gmail.com</a>>> wrote: On Tue, Aug 11, 2015 at 1:34 AM, Jan Cholasta <<a href="mailto:jcholast@redhat.com" target="_blank">jcholast@redhat.com</a> <div><div class="h5"> <mailto:<a href="mailto:jcholast@redhat.com" target="_blank">jcholast@redhat.com</a>>> wrote: On 6.8.2015 21:43, Gabe Alford wrote: Hello, Updated patch attached. - Time limit is -1 for unlimited. I found this <a href="https://www.redhat.com/archives/freeipa-devel/2011-January/msg00330.html" rel="noreferrer" target="_blank">https://www.redhat.com/archives/freeipa-devel/2011-January/msg00330.html</a> in reference to keeping the time limit as -1 for unlimited. This patch does two conflicting things: it coerces time limit of 0 to -1 and at the same time prohibits the user to use 0 for time limit. We should do just one of these and IMHO it should be the coercion of 0 to -1. Sure enough, testing time limit at 0 did not work for unlimited as well as appeared to have negative effects on IPA. This is because the time limit read from ipa config is not converted to int in ldap2.find_entries(), so the coercion does not work. Fix this and 0 will work just fine. Also, I believe that <a href="http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s" rel="noreferrer" target="_blank">http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s</a> specifies unlimited for time limit as -1. (Please correct me if I am wrong.) python-ldap is layers below our API and should not determine what we use for unlimited time limit. I would prefer if we were self-consistent and use 0 for both time limit and size limit. A misunderstanding on my part as I thought it was higher up in the API for some reason. Updated patch attached. </div></div></blockquote> Thanks, this is better, but it turns out I was wrong about coercing -1 to 0 in config-mod: in a topology with different versions of IPA servers, setting the limits in LDAP to 0 on a newer server with your patch will break older servers without your patch: [user@old]$ ipa user-find -------------- 1 user matched -------------- User login: admin Last name: Administrator Home directory: /home/admin Login shell: /bin/bash UID: 1364800000 GID: 1364800000 Account disabled: False Password: True Kerberos keys available: True ---------------------------- Number of entries returned 1 ---------------------------- [user@new]$ ipa config-mod --searchtimelimit=0 --searchrecordslimit=0 ... [user@old]$ ipa user-find --------------- 0 users matched --------------- ---------------------------- Number of entries returned 0 ---------------------------- To fix this, we actually need to do the opposite and store -1 in LDAP when 0 is specified in config-mod options. Honza -- Jan Cholasta </blockquote></div> </div></div></div></div></div>