On 08.06.2016 14:17, Stanislav Laznicka wrote:
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:


please remove it from LDAPAddReverseMember too, it contains the same


Please see the modified patch.



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.

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)?

I still don't think that we should use that attrs list, because according git history, attrs_list was really used only with code that was removed, and as you can see Standa had add extra objectclass attribute there

My 2c

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to