-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
systemd::unit_file: remove hasrestart/hasstatus params #264
Conversation
e29e073
to
76577c3
Compare
@@ -81,23 +75,13 @@ | |||
Optional[Variant[Boolean, Enum['mask']]] $enable = undef, | |||
Optional[Boolean] $active = undef, | |||
Optional[String] $restart = undef, |
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.
Shoudn't the restart
parameter be removed with this as well?
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.
This is the restart
parameter on service
, which I think can still be useful. For example, one could set /usr/bin/systemctl reload-or-restart $unit
.
If you're talking about the one in service_limits
, then I'd agree. IMHO that's a bad pattern.
Boolean $restart_service = true |
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.
systemd::unit_file { "my.service":
....
service_parameters => {
'restart' => "/usr/bin/systemctl reload-or-restart $unit"
}
}
is enough. .. Or is intention to just not add every single option that could possibly go to service - also makes sense.
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.
Oh that's an excellent point. I think overriding this is obscure enough that it's valid to not have a dedicated parameter. 👍
Since these are deprecated in the future do not add them here. voxpupuli#264
Since these are deprecated in the future do not add them here. voxpupuli#264
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.
This seems reasonable to me. I will remove the REFERENCE.md changes from the branch to resolve the merge conflict.
Doh, I just noticed this PR is from a fork. |
This removes two parameters from systemd::unit_file. In release 3.8.0 on 2022-03-02 we introcuced the deprecation warnings. We should merge this PR somewhere in the future when we've other breaking changes. Do not merge at the moment!