Skip to content

Conversation

@zipkid
Copy link
Contributor

@zipkid zipkid commented Feb 20, 2018

@zipkid zipkid changed the title Add param for systemd PrivateTmp setting Add param for systemd service options Mar 2, 2018
@zipkid
Copy link
Contributor Author

zipkid commented Mar 2, 2018

I have changed this PR to allow setting any systemd service option instead of adding a param per option.

@zipkid
Copy link
Contributor Author

zipkid commented Mar 2, 2018

I changed rubocop.yaml and set 'TargetRubyVersion' to 2.2 but codeclimate doesn't seem to pick up that change.

@cliffano
Copy link
Contributor

cliffano commented Jan 4, 2019

Hello @bstopp . Any chance of this PR getting merged?
I encounter many occurrences where the default TimeoutStopSec=4min is not enough, hence the SIGINT wouldn't make a difference anyway since AEM doesn't get a chance to gracefully stop.

@bstopp
Copy link
Owner

bstopp commented Jan 10, 2019

The PR only contains an update to remove the PrivateTmp service def. It still contains the TimeoutStopSec declaration here.

So this won't fix your issue @cliffano.

Let me look at this weekend and see we can get a full/arbitrary set of key/values.

@cliffano
Copy link
Contributor

I was under the impression that the TimeoutStopSec could be redeclared again in the key value pairs and overwrites the default TimeoutStopSec=4min , and since systemd_service_options is a hash, the users would be able to inject anything.

Alternatively, can't the systemd_service_options defines a default hash that contains systemd_service_options: { 'TimeoutStopSec' => '4min', 'KillSignal' => 'SIGCONT', 'PrivateTmp' => true } ?

@bdhoine
Copy link
Contributor

bdhoine commented Feb 8, 2019

@bstopp @zipkid @cliffano can we revisit this PR? It would be nice if this go merged. As far as I can tell we need to tackle the last comment. I think it would be nice if the static config like TimeoutStopSec and KillSignal would be indeed replace by defining them in the systemd_service_options config hash.

@zipkid
Copy link
Contributor Author

zipkid commented Feb 8, 2019

@bdhoine I will look at this next week.

@bstopp
Copy link
Owner

bstopp commented Feb 9, 2019

I'll probably close this in favor of #128. Release coming soon.

@bstopp bstopp closed this Feb 9, 2019
@bstopp
Copy link
Owner

bstopp commented Feb 9, 2019

@zipkid @bdhoine @cliffano Thanks for all the feedback and help with this. I had some issues trying to merge this change in, that's why i have a different branch with the updates. I'm also running into some issue with updating all the dev gem dependencies. Many of them have updates that require rewriting of large portions of the module, hence why not Gemfile version updates; look for those soon.

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.

4 participants