-
Notifications
You must be signed in to change notification settings - Fork 272
use npid for pid file support #555
base: master
Are you sure you want to change the base?
Conversation
5a36c12
to
e16f514
Compare
// boolean values result in the default file path being used | ||
// | ||
// paths are relative to the 'HOME' folder | ||
// |
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.
For consistency with other paths in config (ex: ssl key), this should not be relative to HOME folder (+ it's a limitation).
In which cases would that occur ? |
@apmorton thanks a lot for your contrib and sorry for my ashamingly-high delay on the review, I will try to be more reactive on the discussion :-) |
} | ||
|
||
// get the absolute path of the pid file to use | ||
var pidFilePath = path.resolve(Helper.HOME, config.pidFile); |
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.
as you can see here @JocelynDelalande, although paths are relative to HOME
, I use the resolve function, which respects absolute paths when given.
IE, if you specify pidFile: 'shout.pid'
vs pidFile: '/shout.pid'
the first will be resolved to the home path, and the second will be used verbatim
If you notice in my example cron script, I always attempt to start shout - I do not check for the existence of the pid file to control starting shout. Perhaps if shout is already running with the same pid file, and you try to launch it again, you get a pid file creation error message. |
Have you instead considered systemd instead of relying on a pidfile? |
With a generic pidfile approach, any user can configure shout without needing to be root. |
systemd-user is a thing. |
that being said, I don't know much about systemd, which is why it was not my preferred solution. TL;DR; pidfiles work - so does systemd, something about skinning a cat |
Do we want to have to maintain everything about OS integration?! I mean, we have a Dockerfile, this would add npid information to the config file. I also noticed someone PRing for forever, there were mentions of systemd, ... I'd rather see this repo and project about and shout itself and its features, and let the community come up with integration projects. When some of these reach a nice stable point, we can deem them official if needed. @apmorton, would it be possible to have this in a repo you control and maintain? Then we can list interesting integration projects in a wiki page, the documentation repo and/or website, ... What do you guys think? |
I think having pidfile support is pretty OS agnostic... In any case, I sent this PR at the request of @JocelynDelalande #379 (comment) |
Offering some decent, light and cross-system PID writing mechanism does not hurt, IMHO, and some init system can integrate of a program that writes its pid to a file itself. About installers, that's a different question, as we already offer a preferred and cross-OS way : npm installation. About supporting various init systems (although I think that's not what we should be discussing here), I'm not against a contrib dir including systemd/runnit/OSXinit/whatever init scripts. I do agree that it's something more to maintain, but I'm also sure that it's an important thing for a server software, and we better keep an eye on it (+ it's realy light : one text file for each init system).
To sum it up, the pid file creation (this PR) does not goes as far as an init system but allows easier management/scripting around it. @apmorton I pause technical discussion till we agree on whether or not the feature itself has an interest. |
as per: #379 (comment)
--silent
is useful in a cron job script, like this one I use, in order to prevent repeated log entries about failing to create the pid fileI run the above script with cron every minute in order to check for stale pid files, and then try and start shout.
The stale file check is important, because although the file should be removed on a crash, if the process is killed (SIGKILL), or the server itself crashes, you can be left with a stale pid file which would prevent shout from starting.