I think this could be useful for lib389 client.
Do you have a specific idea how it could be implemented ?

An option would be to implement a new control with 'attr' and 'value'. The control would returned a normalized value.
A benefit is would be that not only python application could use it, like sssd.

I think the comment is too vague... Could you please open the issue for this case? And put the issue link/number here additionally. It will make it more trackable. And when we'll fix the issue, we will remove the comment.

It is a bit weird to do an assertion like this. If Config.compare(st1_config, st2_config) returns True, shouldn't we just do?

assert Config.compare(st1_config, st2_config)

I really wonder why we use protected members of the class? Can we make it just compare_exclude if it's intended to use by the client?

You removed the block in dirsrvtests/tests/suites/lib389/config_compare_test.py, could you please remove it in other places too? It'll be nice to clean up stuff as we moving things from lib389 tests. :)

Another thing that will be nice to do - imports clean up (they have a lot of * and some wrong import order).

Otherwise, PR is nice and improving compare functionality is a very helpful thing. Thanks!

I think this could be useful for lib389 client.
Do you have a specific idea how it could be implemented ?
An option would be to implement a new control with 'attr' and 'value'. The control would returned a normalized value.
A benefit is would be that not only python application could use it, like sssd.

There is already a native ldap compare operation IIRC, have a look at https://www.python-ldap.org/en/python-ldap-3.2.0/reference/ldap.html?highlight=compare#ldap.LDAPObject.compare_s

So the next step to this ticket, is to make lib389 compare say "if we are online, use the ldap compare request".

I think the comment is too vague... Could you please open the issue for this case? And put the issue link/number here additionally. It will make it more trackable. And when we'll fix the issue, we will remove the comment.

Sure, I'll open an issue for it.

I really wonder why we use protected members of the class? Can we make it just compare_exclude if it's intended to use by the client?

So in this case, no. The idea was "I want to compare that my two replicas are identical". That means they should have identical ports in cn=config right?

it's only because this test is performed on localhost dual instance that we CAN'T do this, so we have to exclude the settings that are (in this case) different. Normally, they would never need to be excluded.

Cleaned up and fixed based on your comments @spichugi

Groups, UserAccounts, UserAccount are unused imports.

Groups is an unused import

Could you please fix other assertions too? Sorry for the nitpicking. I just want a consistent cleanup :)

All of this is really minor. You have my ack! Thanks!

All comments applied, will push shortly (doing some git mis-management right now :) )

Updated (and thanks for the detailed review)

Probably, should be is False asserstion

The rest looks good. Thank you! Ack!

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3360

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.