[Freeipa-devel] [PATCH 0042] Removed dead code from LDAPRemoveReverseMember

[Stanislav Laznicka]

On 06/07/2016 10:42 AM, Martin Basti wrote:
>
>
> On 07.06.2016 10:43, Jan Cholasta wrote:
>> On 7.6.2016 10:22, Martin Basti wrote:
>>>
>>>
>>> On 07.06.2016 09:07, Jan Cholasta wrote:
>>>> On 6.6.2016 18:29, Martin Basti wrote:
>>>>>
>>>>>
>>>>> On 03.06.2016 14:28, Stanislav Laznicka wrote:
>>>>>> On 06/03/2016 02:19 PM, Martin Basti wrote:
>>>>>>>
>>>>>>> On 03.06.2016 14:13, Stanislav Laznicka wrote:
>>>>>>>> https://fedorahosted.org/freeipa/ticket/5892
>>>>>>>>
>>>>>>>>
>>>>>>> NACK
>>>>>>>
>>>>>>> please remove it from LDAPAddReverseMember too, it contains the 
>>>>>>> same
>>>>>>> code
>>>>>>>
>>>>>>> Martin^2
>>>>>>
>>>>>> Please see the modified patch.
>>>>>>
>>>>>> Standa
>>>>>>
>>>>> ACK
>>>>>
>>>>> Pushed to master: c56d65b064e1e0410c03cf1206816cad4d8d86cc
>>>>
>>>> I think the attrs_list was supposed to be passed to the
>>>> ldap.get_entry() call rather than removed, which would fix that every
>>>> reverse member command always acts like --all was specified.
>>>>
>>> I'm really afraid, what can happen if we put attr_list into get_entry()
>>> instead of '*', because this code were there for 4 years and I don't
>>> feel happy enough to change it now, what we may break.
>>>
>>> Should I revert this commit then and postpone the ticket?
>>
>> It's a bug and should be fixed. The fix is easy so I see no point in 
>> postponing it. I see no reason to be really afraid, I'm pretty sure 
>> that removing the objectclass attribute (which is invisible in the 
>> CLI anyway) from the output of all the 4 commands that use this code 
>> won't break anything.
>>
>
> Ok
It seems that tests expect objectClass to be always returned in derived 
methods. Is that expected behavior? If so, please see the attached 
patch. I wonder if the keys of the passed options should make it to 
attrs_list as well (similarly to LDAPUpdate and LDAPCreate)?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-slaznick-0044-Revert-Removed-dead-code-from-LDAP-Remove-Add-Revers.patch
Type: text/x-patch
Size: 2030 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160608/3903934c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-slaznick-0045-The-LDAP-ReverseMember-shouldn-t-imply-all-is-always.patch
Type: text/x-patch
Size: 1629 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160608/3903934c/attachment-0001.bin>