IMO it does, because saying just "-1 is default" is not entirely correct and "0 is default" would be confusing, as you pointed out. You might say "0 or -1 is unlimited" if you think it's clearer.

On 10.9.2015 18:39, Gabe Alford wrote:
Oops.. replied without the list.

Reason I said -1 is because users might be confused if they enter `ipa
config-mod --searchtimelimit=0`, and both `ipa user-show` and the webui
show -1 instead of 0. I wonder if -1 makes more sense in that regard?
Thoughts? Does "<= 0 is unlimited" make more sense?

Thanks,

Gabe


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

    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>
        <mailto: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>>
                 <mailto: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>>
                      <mailto: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





--
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