get_dn and modify_dn seems weird to me. It sounds like the command will modify entry DN or it will get the entry DN.
As an option, can we have it as get-by-dn (underscores also looks weird I think) or we can have just get/show. I think it is even better because it is short and the user will see the help with it requres DN info.

@spichugi We have a lot of commands that already use this pattern though, but I think you may be right taht it could be confusing. So we have to choose consistency (because lots already implement the action_dn scheme) or to break consistency in this case. Worth some thought ....

@spichugi We have a lot of commands that already use this pattern though, but I think you may be right taht it could be confusing. So we have to choose consistency (because lots already implement the action_dn scheme) or to break consistency in this case. Worth some thought ....

I found only one... backend get-dn.
The rest of the existing commands follow get pattern. And my upcoming Plugin CLI also uses get pattern.
So, for the consistency, I'd say we better stick to get and modify (DN will be specified as an argument)... What do you think?

Let me have a check of the code and a think about it. There are some reasons why we want a dn over a filter component in these cases.

I think that having the arugemnt as say "get --dn blah" or "get ", is potentially confusing as one adds the --dn param, one adds the selector, and it may affect positional arguments.

I'd say better to have a seperate command like get-dn, or if that's isn't clear, D Ithink you said "get-by-dn" which could be better. I don't think we should mix-and-match in our arguments to cli tools (that's how we generated so much tech debt in the project already in commands and apis ....)

I think that having the arugemnt as say "get --dn blah" or "get ", is potentially confusing as one adds the --dn param, one adds the selector, and it may affect positional arguments.
I'd say better to have a seperate command like get-dn, or if that's isn't clear, D Ithink you said "get-by-dn" which could be better. I don't think we should mix-and-match in our arguments to cli tools (that's how we generated so much tech debt in the project already in commands and apis ....)

I agree with you that the optional --dn parameter is not a good choice.
I think get-by-dn is good when we have get-by-name/get-by-cn - so the user can easily destinguish the commands.
When we have just one thing -get DN in the help, it looks better to me than having get-by-dn DN which looks redundant to me.

What do you think?

I think the confusion could be that other commands use "get " rather than "get ", so this violates consistency of the interface. That's why I think get should always be a filter specificaiton for "get william" for example. That's why I'd rather it be seperate.

I think the confusion could be that other commands use "get " rather than "get ", so this violates consistency of the interface. That's why I think get should always be a filter specificaiton for "get william" for example. That's why I'd rather it be seperate.

Ok, I think I see your point.

So just to sum up:

get-by-dn and modify-by-dn is used for DN specific operations.
get SELECTOR used for selector specific operations.

Is that what you mean? (and I agree to it)

Yes, that is what I mean. I'll adjust this patch accordingly. Thanks!

Looks good to me.
One more thing though - could you please make it get-by-dn and modify-by-dn instead of modify_by_dn and get_by_dn?
It is a common approach we use in other CLI commands.

Updated :)

Ack! Thanks!

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

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.