[Freeipa-devel] [PATCHES] 137-144 LDAP code refactoring (Part 3)

[Jan Cholasta]

On 19.2.2013 16:56, Petr Viktorin wrote:
> On 02/19/2013 02:17 PM, Jan Cholasta wrote:
>> On 29.1.2013 10:21, Jan Cholasta wrote:
>>> A patch from this patchset (part 3) causes some of the dns plugin tests
>>> to fail (idnsallowdynupdate is missing in dnszone_add output).
>>>
>>> Honza
>>>
>>
>> Patch 143:
>>
>> +            assert isinstance(entry_or_dn, DN)
>> +            if normalize is None or normalize:
>> +                entry_or_dn = self.normalize_dn(entry_or_dn)
>> +            entry_attrs = dict(entry_attrs)
>>
>> Can you please return LDAPEntry here as well, i.e. replace
>> dict(entry_attrs) with self.make_entry(entry_or_dn, entry_attrs)?
>
> Sure. I tried to keep the old behavior with old usage, but you're right,
> using LDAPEntry here will be better.
>
>> +    def delete_entry(self, entry, normalize=None):
>> +        """Delete entry.
>> +
>> +        The `normalize` argument does nothing when called with a
>> LDAPEntry.
>> +
>> +        The legacy variant is:
>> +            delete_entry(dn, normalize=True)
>> +        """
>>
>> I don't think this is right. We don't need to know any of the attributes
>> of an entry to delete it, just its DN. I think we should keep the DN
>> variant of delete_entry as the primary one.
>>
>
> Makes sense. I made them both primary.
> I didn't bother documenting normalize since your patch will remove that.
>
>
> Updated patch attached; I'll update my repo when I respond to your
> comments on part 4.
>

ACK part 3.

Honza

-- 
Jan Cholasta