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 grafana_user and switch to using flush #283

Merged
merged 1 commit into from
May 25, 2022

Conversation

alexjfisher
Copy link
Member

@alexjfisher alexjfisher commented May 24, 2022

Previously save_user was being called once per changed property and
calls were also being made to update user properties, password and the
admin flag regardless of whether these properties needed updating.

Using flush is more efficient.

  • full_name is fixed (it was previously a parameter instead of a property).
  • When a user is created, is_admin is correctly set in a single Puppet run.
  • Properties can be managed individually.
  • Instead of making password mandatory, (when creating a user), if
    password isn't specified, a random one is used.

Fixes #121

Previously `save_user` was being called once per changed property and
calls were also being made to update user properties, password and the
admin flag regardless of whether these properties needed updating.

Using `flush` is more efficient.

* `full_name` is fixed (it was previously a parameter instead of a property).
* When a user is created, `is_admin` is correctly set in a single Puppet run.
* Properties can be managed individually.
* Instead of making `password` mandatory, (when creating a user), if
  `password` isn't specified, a random one is used.

Fixes voxpupuli#121
@alexjfisher alexjfisher changed the title Replace grafana_user save_user with flush Fix grafana_user and switch to using flush May 25, 2022
@@ -48,79 +51,82 @@ def name
end

def name=(value)
resource[:name] = value
Copy link
Member Author

Choose a reason for hiding this comment

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

These lines didn't actually do anything. resource[:foo] was already value. (The function wouldn't have been called if the property was not set)

@@ -57,7 +57,6 @@ def insync?(_is)
newproperty(:is_admin) do
desc 'Whether the user is a grafana admin'
newvalues(:true, :false)
defaultto :false
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that update_admin_flag is only called if the property has been set, there is no need for a default here (and defaults for properties are generally a bad thing).

@alexjfisher alexjfisher marked this pull request as ready for review May 25, 2022 10:15
@alexjfisher alexjfisher merged commit c1b3289 into voxpupuli:master May 25, 2022
@alexjfisher alexjfisher deleted the issue_121 branch May 25, 2022 14:18
@ekohl ekohl added the bug Something isn't working label Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot create users as Admin
3 participants