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

Fix influxdb user functionality for version 0.9+ #31770

merged 1 commit into from Mar 9, 2016

Conversation

anlutro
Copy link
Contributor

@anlutro anlutro commented Mar 9, 2016

Fixes #30489

- get_list_cluster_admins no longer exists
- user dict keys changed
@justinta
Copy link

justinta commented Mar 9, 2016

Go Go Jenkins!

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!

@rallytime rallytime added Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Mar 9, 2016
rallytime pushed a commit that referenced this pull request Mar 9, 2016
Fix influxdb user functionality for version 0.9+
@rallytime rallytime merged commit fd3610c into saltstack:2015.8 Mar 9, 2016
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.

None yet

3 participants