-
Notifications
You must be signed in to change notification settings - Fork 901
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
Feature/bugfix 1616 #1630
Feature/bugfix 1616 #1630
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of reviews.
I would suggest to find better ways make "all" case similar to old cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test cases and changelog
08d155c
to
ed16fc4
Compare
to describe all the users, keeps previous behavior for a list
Made some change following what we were talking about, keeping same behavior if a list is passed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Rebase the branch.
- None and empty list both have same behavior. Reflect this in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix CI.
@@ -869,10 +869,14 @@ def test_describe_user_scram_credentials_api(): | |||
# Describe User Scram API | |||
a = AdminClient({"socket.timeout.ms": 10}) | |||
|
|||
f = a.describe_user_scram_credentials() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an integration test for the fix that we are deploying i.e. describe all users case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!.
Fixes #1616