I'm not sure about that, I think it should still say 0, because that's what we want to use as the unlimited value. If you insist on including -1 in the docs, maybe we can say "<= 0 is unlimited"?

On 10.9.2015 16:08, Gabe Alford wrote:
Makes sense. I also changed the doc string to reflect -1 as well.
Updated patch attached.

Thanks,

Gabe

On Thu, Sep 10, 2015 at 1:41 AM, Jan Cholasta <jchol...@redhat.com
<mailto:jchol...@redhat.com>> wrote:

    On 4.9.2015 14:43, Gabe Alford wrote:

        Bump for review.

        On Wed, Aug 12, 2015 at 9:32 AM, Gabe Alford
        <redhatri...@gmail.com <mailto:redhatri...@gmail.com>
        <mailto:redhatri...@gmail.com <mailto:redhatri...@gmail.com>>>
        wrote:

             On Tue, Aug 11, 2015 at 1:34 AM, Jan Cholasta
        <jchol...@redhat.com <mailto:jchol...@redhat.com>
             <mailto:jchol...@redhat.com <mailto:jchol...@redhat.com>>>
        wrote:

                 On 6.8.2015 21:43, Gabe Alford wrote:

                     Hello,

                     Updated patch attached.

                     - Time limit is -1 for unlimited. I found this
        https://www.redhat.com/archives/freeipa-devel/2011-January/msg00330.html
                     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
        
http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s
                     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.


    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




--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to