[Freeipa-devel] make-lint failures

[John Dennis]

On 11/30/2011 04:06 AM, Alexander Bokovoy wrote:
> We need to review all .add_s() calls to ensure their correctness.
> .modify_s() are simpler because they always were used with two
> arguments (dn and list of modifications) but in case of .add_s() we
> were abusing it with Entry as a single argument and were relying on
> the magic of wrapper methods to do transformations.
>
> John's patch gives you this:
>      def add_s(self, dn, modlist):
>          if isinstance(dn, Entry):
>              return IPASimpleLDAPObject.add_s(self, dn.dn, dn.toTupleList())
>          else:
>              return IPASimpleLDAPObject.add_s(self, dn, modlist)
>
> but I think it is semantically better to be explicit and use
> .addEntry() here, especially if you have already went and created
> Entry few lines before that.

+1

I agree it's better to use addEntry(). There is a fair amount of abuse 
of the LDAP API in the installer. What I did was a interim step to help 
us pay attention to what we're passing.

If I recall correctly in the DN refactoring work which has been sitting 
in a branch of mine I replaced all calls passing an Entry to add_s() 
with addEntry().

Cleaning up our use of the LDAP API's is high on my list because we've 
abused in so many places which in turn has made doing the DN conversion 
work unnecessarily difficult.

What I think we really need to do is use only one single IPA specific 
API for talking to LDAP, currently that's ldap2, but I guess that's a 
task for another day.

I would see these .add_s() methods as a
> convenience tool for API compliance if you need quickly convert some
> other LDAP-based Python code being merged to the framework.
>
>> Even the Sumit's rebased patch
>> (freeipa-sbose-0008-5-Add-DNS-service-records-for-Windows.patch) does
>> not solve these pylint issues.
> Because it didn't cover all the .add_s() calls. I asked Sumit to
> consider those cases as well.


-- 
John Dennis <jdennis at redhat.com>

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