Skip to content

Commit

Permalink
Fix rails restart issue with Puma
Browse files Browse the repository at this point in the history
- We need to pass the restart command to Puma so that it will use it
  while restarting the server.
- Also made sure that all the options passed by user while starting
  the server are used in the generated restart command so that they will
  be used while restarting the server.
- Besides that we need to remove the server.pid file for the previous running
  server because otherwise Rack complains about it's presence.
- We don't care if the server.pid file does not exist. We only want to delete
  it if it exists.
- This also requires some changes on Puma side which are being tracked
  here - puma/puma#936.
- Fixes #23910.
  • Loading branch information
prathamesh-sonpatki committed Mar 30, 2016
1 parent afba03f commit 9d87ce3
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 1 deletion.
5 changes: 5 additions & 0 deletions railties/CHANGELOG.md
@@ -1,3 +1,8 @@
* Make `rails restart` command work with Puma by passing the restart command
which Puma can use to restart rails server.

*Prathamesh Sonpatki*

* The application generator writes a new file `config/spring.rb`, which tells
Spring to watch additional common files.

Expand Down
7 changes: 6 additions & 1 deletion railties/lib/rails/commands/server.rb
Expand Up @@ -94,7 +94,8 @@ def default_options
environment: (ENV['RAILS_ENV'] || ENV['RACK_ENV'] || "development").dup,
daemonize: false,
caching: nil,
pid: Options::DEFAULT_PID_PATH
pid: Options::DEFAULT_PID_PATH,
restart_cmd: restart_command
})
end

Expand Down Expand Up @@ -130,5 +131,9 @@ def log_to_stdout
Rails.logger.extend(ActiveSupport::Logger.broadcast(console))
end
end

def restart_command
"bin/rails server #{ARGV.join(' ')}"
end
end
end
1 change: 1 addition & 0 deletions railties/lib/rails/dev_caching.rb
Expand Up @@ -15,6 +15,7 @@ def enable_by_file
end

FileUtils.touch 'tmp/restart.txt'
FileUtils.rm_f('tmp/pids/server.pid')
end

def enable_by_argument(caching)
Expand Down
1 change: 1 addition & 0 deletions railties/lib/rails/tasks/restart.rake
Expand Up @@ -2,4 +2,5 @@ desc "Restart app by touching tmp/restart.txt"
task :restart do
FileUtils.mkdir_p('tmp')
FileUtils.touch('tmp/restart.txt')
FileUtils.rm_f('tmp/pids/server.pid')
end
9 changes: 9 additions & 0 deletions railties/test/application/rake/dev_test.rb
Expand Up @@ -29,6 +29,15 @@ def teardown
assert_match(/Development mode is no longer being cached/, output)
end
end

test 'dev:cache removes server.pid also' do
Dir.chdir(app_path) do
FileUtils.mkdir_p("tmp/pids")
FileUtils.touch("tmp/pids/server.pid")
`rake dev:cache`
assert_not File.exist?("tmp/pids/server.pid")
end
end
end
end
end
9 changes: 9 additions & 0 deletions railties/test/application/rake/restart_test.rb
Expand Up @@ -34,6 +34,15 @@ def teardown
assert File.exist?('tmp/restart.txt')
end
end

test 'rake restart removes server.pid also' do
Dir.chdir(app_path) do
FileUtils.mkdir_p("tmp/pids")
FileUtils.touch("tmp/pids/server.pid")
`rake restart`
assert_not File.exist?("tmp/pids/server.pid")
end
end
end
end
end
14 changes: 14 additions & 0 deletions railties/test/commands/server_test.rb
Expand Up @@ -118,4 +118,18 @@ def test_default_options
assert_equal old_default_options, server.default_options
end
end

def test_restart_command_contains_customized_options
original_args = ARGV.dup
args = ["-p", "4567"]
ARGV.replace args

options = Rails::Server::Options.new.parse! args
server = Rails::Server.new options
expected = "bin/rails server -p 4567"

assert_equal expected, server.default_options[:restart_cmd]
ensure
ARGV.replace original_args
end
end

0 comments on commit 9d87ce3

Please sign in to comment.