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

[datadog_users] Don't panic if no users is found #2302

Merged
merged 4 commits into from Mar 13, 2024

Conversation

skarimo
Copy link
Member

@skarimo skarimo commented Feb 29, 2024

Closes: #2292

@skarimo skarimo requested review from a team as code owners February 29, 2024 20:37
@skarimo skarimo changed the title Don't panic if no users is found [datadog_users] Don't panic if no users is found Feb 29, 2024
nkzou
nkzou previously approved these changes Feb 29, 2024
@skarimo skarimo force-pushed the sherz/handle-when-user-not-found branch from f599068 to 3d5202e Compare March 12, 2024 20:17
@skarimo skarimo requested a review from vincentyl March 13, 2024 15:40
@skarimo skarimo merged commit 9882073 into master Mar 13, 2024
10 checks passed
@skarimo skarimo deleted the sherz/handle-when-user-not-found branch March 13, 2024 18:49
Comment on lines +204 to +206
// We already raised an exception if multiple users were found but no exact email match.
// If user is nil at this stage, we can assume a user with the same handle already exists.
// Find the user and raise a helpful error message.
Copy link

Choose a reason for hiding this comment

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

i see there is a check for statuscode 409 at the top, why not return the error message for user already exists with handle there?

Copy link
Contributor

@nkzou nkzou Mar 13, 2024

Choose a reason for hiding this comment

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

At that stage its still recoverable - if the create call 409 conflicts with a disabled user with the same handle and matching email, that user can be re-enabled. Its only an exception if the handle and email of the old user doesn't match, because then we can't resolve which user is being re-enabled.

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.

Plugin crashed while creating a few hundred users
4 participants