Skip to content
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

NSS: Replace notification message by a less scary one #6967

Closed
wants to merge 1 commit into from

Conversation

aplopez
Copy link
Contributor

@aplopez aplopez commented Oct 2, 2023

When there is no override, replace the message "Unable to find primary gid" by "There is no override for this group" which sound less scary and is a little bit clearer for users.

Why two different message depending on the returned error code?
Because ENOENT, although an error from the LDAP/LDB point of view (the requested entry does not exist) is not an error for SSSD. It is legal that the entry doesn't exist. This just means that there is no override, but it is not an error.

Any other error code (except EOK, of course) represents a real error and we log a more meaningful message including the error code.

Why doing this?
@alexey-tikhonov had already changed the debug level from SSSDBG_MINOR_FAILURE to SSSDBG_FUNC_DATA for the message when ENOENT was returned, because users where scared by this message. It seems that wasn't enough and users are still getting scared by this message.

Resolves: https://issues.redhat.com/browse/SSSD-6770

@aplopez aplopez added the Trivial label Oct 2, 2023
@aplopez aplopez marked this pull request as ready for review October 2, 2023 12:51
@andreboscatto andreboscatto requested a review from ikerexxe October 3, 2023 12:36
Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the patch!

src/responder/nss/nss_protocol_grent.c Outdated Show resolved Hide resolved
@aplopez aplopez force-pushed the message branch 2 times, most recently from ee3fa93 to 288c1bd Compare October 4, 2023 18:02
@aplopez aplopez requested a review from sumit-bose October 4, 2023 18:05
Copy link
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

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

Hi,

thanks for the update, ACK.

bye,
Sumit

Replace the message "Unable to find primary gid" by another one that
sounds less scary and is a little bit clearer for users.
@alexey-tikhonov
Copy link
Member

Pushed PR: #6967

  • master
    • 2c59fd2 - NSS: Replace notification message by a less scary one
  • sssd-2-9
    • c6ea805 - NSS: Replace notification message by a less scary one

@aplopez aplopez deleted the message branch November 28, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants