Pretty minor, couldn't this be ri = inst->require_index | inst->require_internalop_index; since we don't distinguish the two in the if condition? Would make the if condition a bit clearer. Not major though.

Besides that one comment, to me this looks like a good change. Would it be worth a ticket for 1.5/2.0 to make this default to on? I think probably there would be some concern about defaulting to on in the middle of the current 1.4 series, but it would be really good to default this to on in the future.

Pretty minor, couldn't this be ri = inst->require_index | inst->require_internalop_index; since we don't distinguish the two in the if condition? Would make the if condition a bit clearer. Not major though.

Well we still have to determine if it's internal op or not. You could reject internal unindexed ops, but still allow client unindexed searches. Seems silly, but it's allowed since there are two separate config settings.

Ohh yeah you're right! Sorry about that :)

The patch look good to me as well. Ack

Just a minor remark. search return a ldap result while update_intergrity is a plugin function that returns a plugin result (SLAPI_PLUGIN_FAILURE or SLAPI_PLUGIN_SUCCESS). I agree that LDAP_SUCCESS=SLAPI_PLUGIN_SUCCESS=0 but here it think SLAPI_PLUGIN_FAILURE is more appropriate than a ldap result.

If we run both tests then this part fails because the users already exist.
Besides that, looks good to me.

Just a minor remark. search return a ldap result while update_intergrity is a plugin function that returns a plugin result (SLAPI_PLUGIN_FAILURE or SLAPI_PLUGIN_SUCCESS). I agree that LDAP_SUCCESS=SLAPI_PLUGIN_SUCCESS=0 but here it think SLAPI_PLUGIN_FAILURE is more appropriate than a ldap result.

The problem is that we need to return the ldap error code so we know why it failed. In the calling code we still return SLAPI_PLUGIN_FAILURE, but we take the error code from update integrity and set that in the pblock RESULT_CODE. Since this is a bit confusing I now pass the parent pblock to update_integrity(), there we update the result code and we do not meddle with the result code.

Please review...

The python part looks good to me.

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/4249

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.