search

crohr / pkgr

Package any app into deb or rpm packages, using heroku buildpacks
http://crohr.me/pkgr/
MIT License
590 stars 66 forks source link

Customizable daemon processes #22

Closed jgarber closed 9 years ago

jgarber commented 10 years ago

Planning to fix and PR this, but wanted your input @crohr. We have more processes in our app than just web and worker that should be daemonized, but those two are hard-coded in pkgr. I was thinking it would be a safe to assume that anything in the Procfile was daemonizable. I would also always daemonize web if it was a process from the Procfile or defaults).

What do you think?

crohr commented 10 years ago

You're definitely right, there should not be any hardcoded list of daemon process names. You can safely remove the #daemon? check used in the Pkgr::Process class. Thanks in advance for the PR!

jgarber commented 10 years ago

Excellent. Though we probably don't want rake and console daemons (they get added from the buildpack defualts), so it might take a little more work than that. I'll check it out next week.

On Tue, Dec 10, 2013 at 3:44 AM, Cyril Rohr notifications@github.comwrote:

You're definitely right, there should not be any hardcoded list of daemon process names. You can safely remove the #daemon? check used in the Pkgr::Process class. Thanks in advance for the PR!

— Reply to this email directly or view it on GitHubhttps://github.com/crohr/pkgr/issues/22#issuecomment-30208119 .

crohr commented 10 years ago

Now I remember why I added this check in the first place :) Maybe the solution is to add a --daemonize option to pkgr that would allow to add more names to the default DAEMON_PROCESSES array? Because if we blacklist console and rake, then it may only be relevant to the Ruby buildpack...

On Dec 10, 2013, at 6:02 PM, Jason Garber notifications@github.com wrote:

Excellent. Though we probably don't want rake and console daemons (they get added from the buildpack defualts), so it might take a little more work than that. I'll check it out next week.

On Tue, Dec 10, 2013 at 3:44 AM, Cyril Rohr notifications@github.comwrote:

You're definitely right, there should not be any hardcoded list of daemon process names. You can safely remove the #daemon? check used in the Pkgr::Process class. Thanks in advance for the PR!

— Reply to this email directly or view it on GitHubhttps://github.com/crohr/pkgr/issues/22#issuecomment-30208119 .

— Reply to this email directly or view it on GitHub.

jgarber commented 10 years ago

True. Blacklisting isn't the right answer. I think it's safe to assume that anything in the Procfile is daemonizable and the web proc (if supplied by the buildpack defaults) is daemonizable. That's my read of how best to keep parity with Heroku. I like the addition of a daemonize parameter as well.

On Tue, Dec 10, 2013 at 1:06 PM, Cyril Rohr notifications@github.comwrote:

Now I remember why I added this check in the first place :) Maybe the solution is to add a --daemonize option to pkgr that would allow to add more names to the default DAEMON_PROCESSES array? Because if we blacklist console and rake, then it may only be relevant to the Ruby buildpack...

On Dec 10, 2013, at 6:02 PM, Jason Garber notifications@github.com wrote:

Excellent. Though we probably don't want rake and console daemons (they get added from the buildpack defualts), so it might take a little more work than that. I'll check it out next week.

On Tue, Dec 10, 2013 at 3:44 AM, Cyril Rohr notifications@github.comwrote:

You're definitely right, there should not be any hardcoded list of daemon process names. You can safely remove the #daemon? check used in the Pkgr::Process class. Thanks in advance for the PR!

— Reply to this email directly or view it on GitHub< https://github.com/crohr/pkgr/issues/22#issuecomment-30208119> .

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/crohr/pkgr/issues/22#issuecomment-30252208 .

waj commented 10 years ago

Is there any plan to fix this already? Why don't let any of the procfile_entries daemonizable by default. Unless someone scales up any of these processes it wont hurt have more than what it's needed, right? And then if the --daemonize option is supplied then use it to filter which entries are used.

crohr commented 10 years ago

@waj good point, the console and rake daemons procfile entries would only become an issue if the user chooses to scale them up. Consequently, I think we could remove the hard-coded daemon? check. Did I miss something else?

crohr commented 9 years ago

Fixed in #49