Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

add change-pgrp option (default true) #331

Closed
wants to merge 1 commit into from

3 participants

@riywo

I want to use foreman in the Procfile.

# Procfile
foreman:   bundle exec foreman start -f Procfile.internal

In the child foreman process, I don't want to change its process group because the parent foreman process will send signal to the parent's process group. So I added --change-pgrp option. The default value is true. If someone wants to use foreman in Procfile, he or she can use --no-change-pgrp option in the Procfile.

# Procfile
foreman:   bundle exec foreman start -f Procfile.internal --no-change-prgp
@jhass

Strong +1 but for another reason: I want to run foreman under systemd which makes the started process the session leader. Now a process can either be a session leader or a process group leader but not both at the same time and so the call errors out with gems/foreman-0.61.0/lib/foreman/engine.rb:42:in setpgrp': Operation not permitted (Errno::EPERM). For my issue I would be too okay with handling that error gracefully though.

@ddollar
Owner

I think to run a foreman app under systemd you'd be better off trying to use/create an exporter for systemd format. You really shouldn't put foreman itself into production, it's not designed with that use in mind.

I don't really like adding another toggle for this. Is there some way we can detect the right thing to do and just always do that?

@jhass

For my issue, as said, just rescue the error and proceed. I don't know what happens if you try run foreman under foreman.

@riywo

For my case, I tried to use on Heroku to run multi processes on a single dyno. But Heroku didn't allow binding another port, so this option didn't work as I expected...

Now I don't have such a plan, so you can close this issue. I totally agree with @ddollar 's idea.

@ddollar
Owner

I'm going to define more narrowly the scope of Foreman which is running processes interactively in development. I believe that the right answer for production is to export into some other format (upstart, systemd, etc) and get Foreman out of the picture. In that spirit, I'm going to close this pull request, as it seems to add complexity for the sake of being able to run Foreman in production.

If you have any questions please feel free to comment and I'd be happy to address them.

@ddollar ddollar closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 23, 2013
  1. @riywo
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 6 deletions.
  1. +6 −5 lib/foreman/cli.rb
  2. +1 −1  lib/foreman/engine.rb
View
11 lib/foreman/cli.rb
@@ -19,11 +19,12 @@ class Foreman::CLI < Thor
desc "start [PROCESS]", "Start the application (or a specific PROCESS)"
- method_option :color, :type => :boolean, :aliases => "-c", :desc => "Force color to be enabled"
- method_option :env, :type => :string, :aliases => "-e", :desc => "Specify an environment file to load, defaults to .env"
- method_option :formation, :type => :string, :aliases => "-m", :banner => '"alpha=5,bar=3"'
- method_option :port, :type => :numeric, :aliases => "-p"
- method_option :timeout, :type => :numeric, :aliases => "-t", :desc => "Specify the amount of time (in seconds) processes have to shutdown gracefully before receiving a SIGKILL, defaults to 5."
+ method_option :color, :type => :boolean, :aliases => "-c", :desc => "Force color to be enabled"
+ method_option :env, :type => :string, :aliases => "-e", :desc => "Specify an environment file to load, defaults to .env"
+ method_option :formation, :type => :string, :aliases => "-m", :banner => '"alpha=5,bar=3"'
+ method_option :port, :type => :numeric, :aliases => "-p"
+ method_option :timeout, :type => :numeric, :aliases => "-t", :desc => "Specify the amount of time (in seconds) processes have to shutdown gracefully before receiving a SIGKILL, defaults to 5."
+ method_option :change_pgrp, :type => :boolean, :default => true, :desc => "Change process group explicitly. Not work with Windows"
class << self
# Hackery. Take the run method away from Thor so that we can redefine it.
View
2  lib/foreman/engine.rb
@@ -39,7 +39,7 @@ def initialize(options={})
#
def start
# Make sure foreman is the process group leader.
- Process.setpgrp unless Foreman.windows?
+ Process.setpgrp if !Foreman.windows? and options[:change_pgrp]
trap("TERM") { puts "SIGTERM received"; terminate_gracefully }
trap("INT") { puts "SIGINT received"; terminate_gracefully }
Something went wrong with that request. Please try again.