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

Fix influxdb user functionality for version 0.9+ #31770

Merged
merged 1 commit into from Mar 9, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions salt/modules/influx.py
Expand Up @@ -221,8 +221,9 @@ def user_list(database=None, user=None, password=None, host=None, port=None):
client = _client(user=user, password=password, host=host, port=port)
if database:
client.switch_database(database)
return client.get_list_users()
return client.get_list_cluster_admins()
if hasattr(client, 'get_list_cluster_admins') and not database:
return client.get_list_cluster_admins()
return client.get_list_users()


def user_exists(
Expand Down Expand Up @@ -262,7 +263,17 @@ def user_exists(
users = user_list(database, user, password, host, port)
if not isinstance(users, list):
return False
return name in [u['name'] for u in users]

for user in users:
# the dict key could be different depending on influxdb version
username = user.get('user', user.get('name'))
if username:
if username == name:
return True
else:
log.warning('Could not find username in user: %s', user)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intended to warn on every single user in the users list? This seems a little noisy. Would it make more sense to move this out of the loop and warn at the end if we haven't returned already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the log.warning should really never happen, which is why I didn't put much thought into whether to put it in the loop or not. In InfluxDB 0.8, user.get('name') will return the username, and in 0.9+, user.get('user') will return the username. I don't know how 0.7 would behave.

If I put the log outside of the loop, I wouldn't be able to log the user dict that doesn't contain the expected keys, and I'd have to add an additional variable - found_missing_user_keys or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok. Sounds good. I didn't realize the else statement would likely never occur. Thanks for the explanation!


return False


def user_create(name, passwd, database=None, user=None, password=None,
Expand Down