On 11/29/2011 05:12 PM, Alexander Bokovoy wrote:
On Tue, 29 Nov 2011, Rob Crittenden wrote:
Seeing some new make-lint failures after the reworking of ipaldap
function signatures;
$ ./make-lint
ipaserver/install/adtrustinstance.py:101: [E1120,
ADTRUSTInstance.__create_samba_user] No value passed for parameter
'modlist' in function call
ipaserver/install/adtrustinstance.py:190: [E1120,
ADTRUSTInstance.__create_samba_domain_object] No value passed for
parameter 'modlist' in function call
ipaserver/install/adtrustinstance.py:198: [E1120,
ADTRUSTInstance.__create_samba_domain_object] No value passed for
parameter 'modlist' in function call
I wonder if the signature needs to be:
def add_s(self, dn, modlist=None):
For the case were dn isn't an Entry we probably need to raise an
exception if modlist is None (or test to see what python-ldap add_s
does).
The original LDAPObject.add_s() has modlist as non-optional argument:
def add_s(self,dn,modlist):
I don't think it is wise to break that API assumption.
In all the cases above it should get .add_s(entry) replaced by
.addEntry(entry).
The reason these failures are shown is because Martin reverted the
earlier version of Sumit's patch that you mistakenly committed. Sumit
has produced new patch already but there is one minor issue in it
(another .add_s() -> .addEntry() replacement needs to be done).
FWIW this is a perfect example of why not using ldap2 in the installer
code is problematic. When the installer code is directly invoking a
native ldap method though inheritance it becomes difficult to pass
different object types to the methods. In IPA there are many places
where we would benefit from using IPA custom classes (e.g. Entry). For
the DN work I would also like to pass DN objects. I had to add an entire
class subclassed from LDAPObject for no purpose other than to support
object conversion on native LDAP methods and then instantiate that class
in the installer conn objects. It would be so much nicer if we had an
abstraction layer that knew about our preferred internal objects
exposing an LDAP interface suited to our needs. We have such an
abstraction, it's called ldap2, but alas the installer code doesn't use
it :-(
--
John Dennis <jden...@redhat.com>
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
