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

Add loginctl_user type/provider #131

Merged
merged 7 commits into from
Jan 7, 2020
Merged

Add loginctl_user type/provider #131

merged 7 commits into from
Jan 7, 2020

Conversation

raphink
Copy link
Member

@raphink raphink commented Jan 6, 2020

Example usage:

loginctl_user { 'foo':
  linger => enabled,
}

@bastelfreak
Copy link
Member

@raphink should this be documented in the README.md?

@bastelfreak bastelfreak merged commit 5cfafaa into voxpupuli:master Jan 7, 2020
@raphink
Copy link
Member Author

raphink commented Jan 7, 2020

Actually, before we release 2.8.0, I'm wondering if this is the right interface for the type. Maybe this would be better:

user_linger { 'foo':
  ensure => enabled,
}

It makes the type ensurable and removes the implementation (loginctl) from the type.

What do you think @bastelfreak, @mcanevet?

@bastelfreak
Copy link
Member

From a user point of view I think both ways are. The implementation for your user_linger example might be easier. Another idea I had is that another parameter to the user core type would be nice. but getting stuff into puppet core takes a long time...

@trevor-vaughan
Copy link
Contributor

Given how many users/resources there may be on a given system, you may want to have a Hash interface that only creates one resource to prevent death by 10k resources. Probably future work, but something to keep in mind before this completely goes down the file_line/augeas rabbit hole.

@raphink
Copy link
Member Author

raphink commented Jan 7, 2020

Yes, the core type was an option, too. That would require adding a new feature also. It is probably possible to provide this as a plugin with some monkey patching... but supporting that on the long run might not be fun... and does it make sense to implement linger for all OSes?

@trevor-vaughan
Copy link
Contributor

Looking at the options a bit more, I would prefer the current implementation since it reduces system resources. Technically, you could implement both and make them conflict with each other but that may be too much work.

@raphink
Copy link
Member Author

raphink commented Jan 7, 2020

@trevor-vaughan why does it reduce system resources?

@trevor-vaughan
Copy link
Contributor

trevor-vaughan commented Jan 7, 2020

@raphink Because loginctl_user is one resource with many future options whereas user_linger is one resource per option. In the OO model, the user is the object and linger is the attribute. If you start modeling based on attributes, it explodes quickly.

I still think it's a horrible idea to support different daemons with different versions in the same module, but I lost that fight a while ago. Looks like this changed a while ago and I didn't keep up so they're all in lock-step now for the future!

@raphink
Copy link
Member Author

raphink commented Jan 7, 2020

Good point... so let's keep it the way it is then. I'll cut the release tomorrow I think.

@raphink raphink added the enhancement New feature or request label Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants