On 30.11.2015 08:19, Jan Cholasta wrote:
On 24.11.2015 10:15, Martin Basti wrote:
On 20.11.2015 09:00, Jan Cholasta wrote:
On 19.11.2015 14:13, Jan Cholasta wrote:
On 19.11.2015 14:09, Martin Babinsky wrote:
On 11/19/2015 01:08 PM, Martin Basti wrote:
On 18.11.2015 14:26, Martin Basti wrote:
On 18.11.2015 14:24, Martin Kosek wrote:
On 11/18/2015 02:18 PM, Martin Basti wrote:
On 18.11.2015 13:55, Martin Kosek wrote:
On 11/18/2015 01:30 PM, Martin Basti wrote:
+ DEFAULT_TIME_LIMIT = -1.0
+ DEFAULT_SIZE_LIMIT = 0
...
if time_limit is None or time_limit == 0:
- time_limit = -1.0
+ if self.time_limit is not None and self.time_limit
!= 0:
+ time_limit = self.time_limit
+ else:
+ time_limit = self.DEFAULT_TIME_LIMIT
+
I wonder why is the -1 default time limit a float number, I
would
expect that
some trouble may emerge out of it. Maybe Rob knows?
Python LDAP allows to have smaller granularity than seconds
and it
internally
convert time limit values to float.
Hm, ok. I was just curious since the value we expose in API is Int
and the
default does not use the smaller granularity, so I was thinking
"Why
bother?".
If it is fixed in your patch, good. If not, good also, no need to
bikeshed here
I suppose.
I just keep the original values there, I do not want to touch it
with
this patch
Updated patch attached, the original one has wrong default limit for
ldap2
ACK
Hold your horses.
1) The upgrade_ldapsearch_limit option is not very useful, either add
generic search_time_limit and search_records_limit options, or don't
add any options at all (which, incidentally, is exactly what the
ticket asks for).
2) The limits are not passed to the connection using __init__()
anywhere, so there is no need to have arguments for them.
3) Rather than adding layers of if statements everywhere, implement
the time_limit and size_limit attributes as properties.
4) Use config_entry.single_value when setting defaults in
ldap2.get_ipa_config().
BTW, add comments only when it's not obvious what the code does - a
comment saying "set defaults" above two .setdefault() calls is pretty
redundant.
Updated patch attached
In ldap2.create_connection(), you should set self.time_limit only if
time_limit is not None, ditto for size limit, instead of handling None
in the properties' setters.
You don't need to handle the limits in ldap2.find_entries() anymore,
since it is handled by LDAPClient.find_entries(), so you can remove
the related code completely from ldap2.find_entries().
Updated patch attached.
From de56acb8b745dab14560790b4166c852ad014651 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 18 Nov 2015 10:31:05 +0100
Subject: [PATCH] Upgrade: increase time limit for upgrades
Default ldap search limit is now 30 sec by default during upgrade.
Limits must be changed for the whole ldap2 connection, because this
connection is used inside update plugins and commands called from
upgrade.
Together with increasing the time limit, also size limit should be
unlimited during upgrade. With sizelimit=None we may get the
TimeExceeded exception from getting default value of the sizelimit from LDAP.
https://fedorahosted.org/freeipa/ticket/5267
---
ipalib/plugins/permission.py | 3 +-
ipapython/ipaldap.py | 10 ++++--
ipaserver/install/ldapupdate.py | 5 ++-
ipaserver/plugins/ldap2.py | 70 ++++++++++++++++++++++++-----------------
4 files changed, 55 insertions(+), 33 deletions(-)
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index b17b61e696a8f333a25150f60f27cffd03085196..cbc48f5e8a6e5765d5eca59797ca75b7a9d98244 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -1292,8 +1292,7 @@ class permission_find(baseldap.LDAPSearch):
if 'sizelimit' in options:
max_entries = options['sizelimit']
else:
- config = ldap.get_ipa_config()
- max_entries = int(config.single_value['ipasearchrecordslimit'])
+ max_entries = self.api.Backend.ldap2.size_limit
filters = ['(objectclass=ipaPermission)',
'(!(ipaPermissionType=V2))']
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 7e5bc04fea1dc89cc141051025d00a57d74b2b41..8274039e6d6b6cad431ea83e9bc768afadca77ab 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -712,6 +712,8 @@ class LDAPClient(object):
self._force_schema_updates = force_schema_updates
self._no_schema = no_schema
self._decode_attrs = decode_attrs
+ self.time_limit = -1.0 # unlimited
+ self.size_limit = 0 # unlimited
self.log = log_mgr.get_logger(self)
self._has_schema = False
@@ -1294,10 +1296,14 @@ class LDAPClient(object):
res = []
truncated = False
- if time_limit is None or time_limit == 0:
+ if time_limit is None:
+ time_limit = self.time_limit
+ if time_limit == 0:
time_limit = -1.0
+
if size_limit is None:
- size_limit = 0
+ size_limit = self.size_limit
+
if not isinstance(size_limit, int):
size_limit = int(size_limit)
if not isinstance(time_limit, float):
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 86c01104710ff166a07057f9873ed8661621e704..2ed5e8f4e656f11a8de696d6a3bf08bcfac45d47 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -46,6 +46,7 @@ from ipapython.ipa_log_manager import *
from ipapython.ipautil import wait_for_open_socket
UPDATES_DIR=paths.UPDATES_DIR
+UPDATE_SEARCH_TIME_LIMIT = 30 # seconds
def connect(ldapi=False, realm=None, fqdn=None, dm_password=None, pw_name=None):
@@ -867,7 +868,9 @@ class LDAPUpdate:
self.api.Backend.ldap2.connect(
bind_dn=DN(('cn', 'Directory Manager')),
bind_pw=self.dm_password,
- autobind=self.ldapi)
+ autobind=self.ldapi,
+ time_limit=UPDATE_SEARCH_TIME_LIMIT,
+ size_limit=0)
self.conn = self.api.Backend.ldap2
else:
raise RuntimeError("Offline updates are not supported.")
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 4cdc561660a83ebbd6b0bf2f24c97c42ac0a9b20..a1f1e1982980f1a913d7f5f40ee4e7c93cff64e7 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -72,6 +72,39 @@ class ldap2(CrudBackend, LDAPClient):
LDAPClient.__init__(self, ldap_uri,
force_schema_updates=force_schema_updates)
+ self.__time_limit = None
+ self.__size_limit = None
+
+ @property
+ def time_limit(self):
+ if self.__time_limit is None:
+ return float(self.get_ipa_config().single_value.get(
+ 'ipasearchtimelimit', 2))
+ return self.__time_limit
+
+ @time_limit.setter
+ def time_limit(self, val):
+ self.__time_limit = float(val)
+
+ @time_limit.deleter
+ def time_limit(self):
+ self.__time_limit = None
+
+ @property
+ def size_limit(self):
+ if self.__size_limit is None:
+ return int(self.get_ipa_config().single_value.get(
+ 'ipasearchrecordslimit', 0))
+ return self.__size_limit
+
+ @size_limit.setter
+ def size_limit(self, val):
+ self.__size_limit = int(val)
+
+ @size_limit.deleter
+ def size_limit(self):
+ self.__size_limit = None
+
def _connect(self):
# Connectible.conn is a proxy to thread-local storage;
# do not set it
@@ -87,7 +120,7 @@ class ldap2(CrudBackend, LDAPClient):
def create_connection(self, ccache=None, bind_dn=None, bind_pw='',
tls_cacertfile=None, tls_certfile=None, tls_keyfile=None,
debug_level=0, autobind=AUTOBIND_AUTO, serverctrls=None,
- clientctrls=None):
+ clientctrls=None, time_limit=None, size_limit=None):
"""
Connect to LDAP server.
@@ -114,6 +147,11 @@ class ldap2(CrudBackend, LDAPClient):
if tls_keyfile is not None:
_ldap.set_option(_ldap.OPT_X_TLS_KEYFILE, tls_keyfile)
+ if time_limit is not None:
+ self.time_limit = time_limit
+ if size_limit is not None:
+ self.size_limit = size_limit
+
if debug_level:
_ldap.set_option(_ldap.OPT_DEBUG_LEVEL, debug_level)
@@ -175,31 +213,9 @@ class ldap2(CrudBackend, LDAPClient):
# ignore when trying to unbind multiple times
pass
- def find_entries(self, filter=None, attrs_list=None, base_dn=None,
- scope=_ldap.SCOPE_SUBTREE, time_limit=None,
- size_limit=None, search_refs=False, paged_search=False):
+ del self.time_limit
+ del self.size_limit
- def _get_limits():
- """Get configured global limits, caching them for more calls"""
- if not _lims:
- config = self.get_ipa_config()
- _lims['time'] = int(config.get('ipasearchtimelimit', [None])[0])
- _lims['size'] = int(config.get('ipasearchrecordslimit', [None])[0])
- return _lims
- _lims = {}
-
- if time_limit is None:
- time_limit = _get_limits()['time']
- if size_limit is None:
- size_limit = _get_limits()['size']
-
- res, truncated = super(ldap2, self).find_entries(
- filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
- time_limit=time_limit, size_limit=size_limit,
- search_refs=search_refs, paged_search=paged_search)
- return (res, truncated)
-
- config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]}
def get_ipa_config(self, attrs_list=None):
"""Returns the IPA configuration entry (dn, entry_attrs)."""
@@ -223,9 +239,7 @@ class ldap2(CrudBackend, LDAPClient):
config_entry = entries[0]
except errors.NotFound:
config_entry = self.make_entry(dn)
- for a in self.config_defaults:
- if a not in config_entry:
- config_entry[a] = self.config_defaults[a]
+
context.config_entry = config_entry
return config_entry
--
2.5.0
--
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
