Skip to content

Issue 6067 - Improve dsidm CLI No Such Entry handling #6079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

droideck
Copy link
Member

@droideck droideck commented Feb 7, 2024

Add additional error processing to dsidm CLI tool for when basedn or OU subentries are absent.

Related: #6067

Reviewed by: ?

Copy link
Member

@vashirov vashirov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still a bit awkward that it allows to enter some details only to fail later:

dsidm localhost -b dc=example,dc=com service create
Enter value for cn : test
Enter value for description : test
Error: The base DN "ou=Services,dc=example,dc=com" does not exist

But the new error message is an improvement :)
LGTM

@droideck
Copy link
Member Author

It's still a bit awkward that it allows to enter some details only to fail later:

dsidm localhost -b dc=example,dc=com service create
Enter value for cn : test
Enter value for description : test
Error: The base DN "ou=Services,dc=example,dc=com" does not exist

But the new error message is an improvement :) LGTM

Okay, it was actually another typo in the same code area (services instead of service).
Now it should correctly display Error: The DN "ou=Services,dc=example,dc=com" does not exist. It is required for "service" subcommand. Please create it first. error.

Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: 389ds#6067

Reviewed by: ?
@droideck droideck merged commit d379ac9 into 389ds:main Feb 16, 2024
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
droideck added a commit that referenced this pull request Feb 16, 2024
Description: Add additional error processing to dsidm CLI tool for when basedn
or OU subentries are absent.

Related: #6067

Reviewed by: @vashirov (Thanks!)
@Firstyear
Copy link
Contributor

Sorry to re-open this one, but shouldn't

def _get_basedn_arg(inst, args, log, msg=None):
be reading the basedn from dsrc as well? Else you always need it with -b.

@progier389
Copy link
Contributor

Hi @Firstyear .
IMHO, it would change the current behavior:
If I understood rightly in current code the base dn is interactively asked if -b option is missing (like for other dsidm mandatory options).
So if we use a default value from dsrc there will be no way to interactively ask for values
unless we choose to drastically change the interface (for example by adding an interactive mode option)
I have no strong feeling about that (on the plus side: it would make dsidm more like the other dsxxx commands,
but on the minus side it will change the interface and may disrupt some customer scripts/habits.)
In any case, if we want to pursue this idea, I think it is unrelated to the original issue and deserves its own ticket.
What do you think ?

@Firstyear
Copy link
Contributor

Firstyear commented Oct 16, 2024

But if the default value is set, it's assumed the admin wants to use that unless they override it with -b?

We had a customer report this as a regression in behaviour, which is why I bring it up :)

edit: https://bugzilla.suse.com/show_bug.cgi?id=1231462

@droideck
Copy link
Member Author

But if the default value is set, it's assumed the admin wants to use that unless they override it with -b?

We had a customer report this as a regression in behaviour, which is why I bring it up :)

edit: https://bugzilla.suse.com/show_bug.cgi?id=1231462

I agree. I think it should be [check if -b provided] -> if not -> [check if basedn is in .dsrc] -> if not -> [ask interactively].
I'll prepare PR, it's an easy fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants