Sounds good to me. Updated patch attached. On Mon, Sep 14, 2015 at 1:34 AM, Petr Spacek <pspa...@redhat.com> wrote:
> On 14.9.2015 07:23, Jan Cholasta wrote: > > 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. > > my +1 to "0 or -1 is unlimited" variant > > Petr^2 Spacek > > > > 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 > > -- > 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 >
From 99070f93a51c7e03fa9c98b3548420fc589eddc1 Mon Sep 17 00:00:00 2001 From: Gabe <redhatri...@gmail.com> Date: Mon, 14 Sep 2015 06:56:00 -0600 Subject: [PATCH] Standardize minvalue for ipasearchrecordlimit and ipasesarchsizelimit for unlimited minvalue https://fedorahosted.org/freeipa/ticket/4023 --- install/ui/test/data/ipa_init_commands.json | 6 +++--- install/ui/test/data/ipa_init_objects.json | 6 +++--- install/ui/test/data/json_metadata.json | 4 ++-- ipalib/plugins/baseldap.py | 4 ++-- ipalib/plugins/config.py | 19 ++++++++++++------- ipaserver/plugins/ldap2.py | 4 ++-- 6 files changed, 24 insertions(+), 19 deletions(-) diff --git a/install/ui/test/data/ipa_init_commands.json b/install/ui/test/data/ipa_init_commands.json index 743f508e2a733b766008bdd21838454ef7df8c21..c7f717c357624489d0b7f43fdd01b5bb8b1bcd86 100644 --- a/install/ui/test/data/ipa_init_commands.json +++ b/install/ui/test/data/ipa_init_commands.json @@ -2446,7 +2446,7 @@ "attribute": true, "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)", + "doc": "Maximum amount of time (seconds) for a search (-1 or 0 is unlimited)", "flags": [ "nonempty" ], @@ -2460,7 +2460,7 @@ "attribute": true, "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum number of records to search (-1 is unlimited)", + "doc": "Maximum number of records to search (-1 or 0 is unlimited)", "flags": [ "nonempty" ], @@ -24018,4 +24018,4 @@ "methods": {}, "objects": {} } -} \ No newline at end of file +} diff --git a/install/ui/test/data/ipa_init_objects.json b/install/ui/test/data/ipa_init_objects.json index c8c836926d94dd4c1903aa9a62fa91c11a238e75..ca98a1a22855bfcc306e1a3ed98e398f1b4505b1 100644 --- a/install/ui/test/data/ipa_init_objects.json +++ b/install/ui/test/data/ipa_init_objects.json @@ -498,7 +498,7 @@ { "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)", + "doc": "Maximum amount of time (seconds) for a search (-1 or 0 is unlimited)", "flags": [], "label": "Search time limit", "maxvalue": 2147483647, @@ -510,7 +510,7 @@ { "class": "Int", "deprecated_cli_aliases": [], - "doc": "Maximum number of records to search (-1 is unlimited)", + "doc": "Maximum number of records to search (-1 or 0 is unlimited)", "flags": [], "label": "Search size limit", "maxvalue": 2147483647, @@ -9158,4 +9158,4 @@ } } } -} \ No newline at end of file +} diff --git a/install/ui/test/data/json_metadata.json b/install/ui/test/data/json_metadata.json index a9e66230f94875f7da7efa73141e244b8201a5cc..5365336febeffc78e8878745a20e5e81f240b38a 100644 --- a/install/ui/test/data/json_metadata.json +++ b/install/ui/test/data/json_metadata.json @@ -2596,7 +2596,7 @@ "cli_name": "searchtimelimit", "cli_short_name": null, "default": null, - "doc": "Max. amount of time (sec.) for a search (> 0, or -1 for unlimited)", + "doc": "Max. amount of time (sec.) for a search (-1 or 0 is unlimited)", "exclude": null, "flags": [], "hint": null, @@ -2619,7 +2619,7 @@ "cli_name": "searchrecordslimit", "cli_short_name": null, "default": null, - "doc": "Max. number of records to search (-1 is unlimited)", + "doc": "Max. number of records to search (-1 or 0 is unlimited)", "exclude": null, "flags": [], "hint": null, diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 31f38946afcbb51638bcab68a6e74ec309eba0e4..dbf6c5da1c41ccfc043876e35299beab96305d42 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -1918,14 +1918,14 @@ class LDAPSearch(BaseLDAPCommand, crud.Search): takes_options = ( Int('timelimit?', label=_('Time Limit'), - doc=_('Time limit of search in seconds'), + doc=_('Time limit of search in seconds (-1 or 0 is unlimited)'), flags=['no_display'], minvalue=0, autofill=False, ), Int('sizelimit?', label=_('Size Limit'), - doc=_('Maximum number of entries returned'), + doc=_('Maximum number of entries returned (-1 or 0 is unlimited)'), flags=['no_display'], minvalue=0, autofill=False, diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py index 6267313d5e9af2d97f45f987115de143d7aa7915..9a090953a1b80ff27acce6487238ebbd038d60de 100644 --- a/ipalib/plugins/config.py +++ b/ipalib/plugins/config.py @@ -78,11 +78,6 @@ EXAMPLES: register = Registry() -def validate_searchtimelimit(ugettext, limit): - if limit == 0: - raise ValidationError(name='ipasearchtimelimit', error=_('searchtimelimit must be -1 or > 1.')) - return None - @register() class config(LDAPObject): """ @@ -150,10 +145,10 @@ class config(LDAPObject): label=_('Default e-mail domain'), doc=_('Default e-mail domain'), ), - Int('ipasearchtimelimit', validate_searchtimelimit, + Int('ipasearchtimelimit', cli_name='searchtimelimit', label=_('Search time limit'), - doc=_('Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)'), + doc=_('Maximum amount of time (seconds) for a search (-1 is unlimited)'), minvalue=-1, ), Int('ipasearchrecordslimit', @@ -267,6 +262,16 @@ class config_mod(LDAPUpdate): name=k, error=_('attribute "%s" not allowed') % a ) + # Set ipasearchrecordslimit to -1 if 0 is used + if 'ipasearchrecordslimit' in entry_attrs: + if entry_attrs['ipasearchrecordslimit'] is 0: + entry_attrs['ipasearchrecordslimit'] = -1 + + # Set ipasearchtimelimit to -1 if 0 is used + if 'ipasearchtimelimit' in entry_attrs: + if entry_attrs['ipasearchtimelimit'] is 0: + entry_attrs['ipasearchtimelimit'] = -1 + for (attr, obj) in (('ipauserobjectclasses', 'user'), ('ipagroupobjectclasses', 'group')): if attr in entry_attrs: diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index deb0592ab68ab8eb712a6d29fdffd8776e2e289a..dced93784771f794759b01e8dbaafe3e659da80c 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -179,8 +179,8 @@ class ldap2(CrudBackend, LDAPClient): """Get configured global limits, caching them for more calls""" if not _lims: config = self.get_ipa_config() - _lims['time'] = config.get('ipasearchtimelimit', [None])[0] - _lims['size'] = config.get('ipasearchrecordslimit', [None])[0] + _lims['time'] = int(config.get('ipasearchtimelimit', [None])[0]) + _lims['size'] = int(config.get('ipasearchrecordslimit', [None])[0]) return _lims _lims = {} -- 1.8.3.1
-- 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
