[Freeipa-devel] make-lint failures

[John Dennis]

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 <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/