It is better to compare it with None. More safe

It is better to compare it with None. More safe

The original Issue considers e.g. an empty string to be the same case as None. Therefore, the suggestion would be invalid. However, a question would be if the original request is itself valid; IDK.

@spichugi if we compare with None . it will make it

    if rdn is None:
        self._basedn = basedn
    else:
        self._basedn = '{},{}'.format(rdn, basedn)

(Pdb) PosixGroups(topo_m4.all_insts.get('master1'), SUBSUFFIX, rdn=None)._basedn
'dc=SubSuffix,dc=autoMembers,dc=com' ---- right
(Pdb) PosixGroups(topo_m4.all_insts.get('master1'), SUBSUFFIX, rdn='')._basedn
',dc=SubSuffix,dc=autoMembers,dc=com' ---- wrong

with the current version:

    if rdn:
        self._basedn = '{},{}'.format(rdn, basedn)
    else:
        self._basedn = basedn

(Pdb) PosixGroups(topo_m4.all_insts.get('master1'), SUBSUFFIX, rdn=None)._basedn
'dc=SubSuffix,dc=autoMembers,dc=com' ---- right
(Pdb) PosixGroups(topo_m4.all_insts.get('master1'), SUBSUFFIX, rdn='')._basedn
'dc=SubSuffix,dc=autoMembers,dc=com' ---- right

so we have to go with current version

@spichugi changes are done , as per pour suggestion .

Please, fix other groups.py functionality too as stated in the issue.

@spichugi changes are done

I think it is better to preserve ensure_str(rdn), ensure_str(basedn) part because it is safer.
Sorry if I created confusion by me first comment here.

@spichugi changes are done

You haven't applied the same logic for PosixGroups... Was it intentional?

You haven't applied the same logic for PosixGroups... Was it intentional?

ensure_str was not imported there in PosixGroups before , now i have imported and applied same logic there also .

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

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.