[Freeipa-devel] [PATCHES] 213-224 Use old entry state in LDAP mods

Wed Dec 18 11:50:53 UTC 2013

On 11.12.2013 18:28, Petr Viktorin wrote:
> On 12/11/2013 05:05 PM, Petr Viktorin wrote:
>> On 12/10/2013 02:10 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> the attached patches fix <https://fedorahosted.org/freeipa/ticket/3488>.
>>>
>>> Honza
>>
>> These look great, thanks! Just a couple of questions/nicpicks.
>>
>> 213: ACK
>> 214: ACK
>> 215: ACK
>> 216: ACK
>> 217: ACK
>> 218: ACK
>>
>> 219:
>> Does the new method guarantee 'attributetypes' are updated before
>> 'objectclasses'?
>> I can move the logic to schemaupdate if needed.

That won't be necessary, fixed.

>>
>> Why is OnlyOneValueAllowed not raised any more? It should be, since the
>> method assumes get_single_value() gives correct information.

Fixed.

>>
>> Why is MOD_REPLACE no longer used when no old values are retained? Or
>> (MOD_DELETE, a, None) when the new set is empty?

Fixed.

However, I am not convinced if this guessing is the right approach. I 
think we can do better, by either not guessing:

     del entry[attr] -> delete attribute
     entry[attr] = value -> replace attribute
     entry[attr][index] = value -> delete old value, add new value

or by guessing better, to minimize the size of the modify request.

>>
>>
>> 220: ACK
>>
>> 221:
>> An ancient comment in ldapupdate still has a reference to orig_data in,
>> can you remove the comment as well?

Sure.

>>
>> 222: ACK
>> 223: ACK
>> 224: ACK
>
> I spoke too soon, NACK. This line:
>
>  > -from ipaserver.plugins import ldap2
>
> is important, please put it back. Without it api.Backend.ldap2 will not
> be available and upgrades will fail with AttributeError.
>
> (Yes, I know.)

Fixed. (But it's retarded.)

>
>
>> I noticed that IPASimpleLdapObject.convert_result's docstring is
>> obsolete, could you update/shorten it?

Sure.

Also fixed some more issues I noticed.

Updated patches attached.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-213.1-Rename-LDAPEntry-method-commit-to-reset_modlist.patch
Type: text/x-patch
Size: 1237 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-214.1-Use-old-entry-state-in-LDAPClient.update_entry.patch
Type: text/x-patch
Size: 4718 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-215.1-Move-LDAPClient-method-get_single_value-to-IPASimple.patch
Type: text/x-patch
Size: 3089 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-216.1-Make-IPASimpleLDAPObject.get_single_value-result-ove.patch
Type: text/x-patch
Size: 1969 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-217.1-Use-LDAPClient.update_entry-for-LDAP-mods-in-ldapupd.patch
Type: text/x-patch
Size: 4389 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-218.1-Reduce-amount-of-LDAPEntry.reset_modlist-calls-in-ld.patch
Type: text/x-patch
Size: 1976 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-219.1-Add-LDAPEntry-method-generate_modlist.patch
Type: text/x-patch
Size: 5983 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-220.1-Remove-unused-LDAPClient-methods-get_syntax-and-get_.patch
Type: text/x-patch
Size: 1415 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-221.1-Remove-legacy-LDAPEntry-properties-data-and-orig_dat.patch
Type: text/x-patch
Size: 4015 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-222.1-Store-old-entry-state-in-dict-rather-than-LDAPEntry.patch
Type: text/x-patch
Size: 4471 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-223.1-Do-not-crash-on-bad-LDAP-data-when-formatting-decode.patch
Type: text/x-patch
Size: 1003 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-224.1-Use-raw-LDAP-data-in-ldapupdate.patch
Type: text/x-patch
Size: 3242 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131218/9b55ec25/attachment-0011.bin>