Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Committed improvements #35

Merged
merged 12 commits into from
Apr 10, 2018
9 changes: 9 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changes

## master

* Add `:pumactl` option
* Don't stop Puma if it was not started by Guard
* Don't notify about start when no start
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

* Improve `guard-compat` using (https://github.com/guard/guard-compat#migrating-your-api-calls)
* Remove unused `pry` dependency
* Update versions of dependencies

## 0.4.1

* Improve notifications. Via #30
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ end

* `:port` is the port number to run on (default `4000`)
* `:environment` is the environment to use (default `development`)
* `:start_on_start` will start the server when starting Guard (default `true`)
* `:start_on_start` will start the server when starting Guard and stop the server when reloading/stopping Guard (default `true`)
* `:force_run` kills any process that's holding open the listen port before attempting to (re)start Puma (default `false`).
* `:daemon` runs the server as a daemon, without any output to the terminal that ran `guard` (default `false`).
* `:quiet` runs the server in quiet mode, suppressing output (default `true`).
Expand All @@ -44,6 +44,9 @@ end
* `:control_token` is the token to use as authentication for the control server(optional)
* `:control_port` is the port to use for the control server(optional)
* `:threads` is the min:max number of threads to use. Defaults to 0:16 (optional)
* `:pumactl` manages the server via `pumactl` executable instead of `puma` (default `false`)
* Incompatible with options such as `port`, `environment`, `daemon`, `bind`, `threads`
* Use with `config` option is preferred.
* `:notifications` is the list of notification types that will be sent. Defaults to `[:restarting, :restarted, :not_restarted, :stopped]` (optional)

## Contributing
Expand Down
7 changes: 3 additions & 4 deletions guard-puma.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ Gem::Specification.new do |gem|
gem.add_dependency "guard", "~> 2.14"
gem.add_dependency "guard-compat", "~> 1.2"
gem.add_dependency "puma", "~> 3.6"
gem.add_development_dependency "rake", "~> 10.4"
gem.add_development_dependency "rspec", "~> 3.5.0"
gem.add_development_dependency "guard-rspec", "~> 4.7.0"
gem.add_development_dependency "pry"
gem.add_development_dependency "rake", "~> 12"
gem.add_development_dependency "rspec", "~> 3.7"
gem.add_development_dependency "guard-rspec", "~> 4.7"
end
44 changes: 29 additions & 15 deletions lib/guard/puma.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
require "guard"
require "guard/plugin"
require "guard/puma/runner"
require "rbconfig"
require "guard/puma/version"
require "guard/compat/plugin"
require 'guard/compat/plugin'
require_relative 'puma/runner'
require_relative 'puma/version'

module Guard
class Puma < Plugin
Expand All @@ -14,6 +11,7 @@ def self.default_env
end

DEFAULT_OPTIONS = {
:pumactl => false,
:port => 4000,
:environment => default_env,
:start_on_start => true,
Expand All @@ -31,32 +29,48 @@ def initialize(options = {})
end

def start
return unless options[:start_on_start]
server = options[:server] ? "#{options[:server]} and " : ""
UI.info "Puma starting#{port_text} in #{server}#{options[:environment]} environment."
runner.start if options[:start_on_start]
Compat::UI.info(
"Puma starting#{port_text} in #{server}#{options[:environment]} environment."
)
runner.start
end

def reload
UI.info "Restarting Puma..."
Compat::UI.info "Restarting Puma..."
if options[:notifications].include?(:restarting)
Notifier.notify("Puma restarting#{port_text} in #{options[:environment]} environment...", :title => "Restarting Puma...", :image => :pending)
Compat::UI.notify(
"Puma restarting#{port_text} in #{options[:environment]} environment...",
title: "Restarting Puma...", image: :pending
)
end
if runner.restart
UI.info "Puma restarted"
Compat::UI.info "Puma restarted"
if options[:notifications].include?(:restarted)
Notifier.notify("Puma restarted#{port_text}.", :title => "Puma restarted!", :image => :success)
Compat::UI.notify(
"Puma restarted#{port_text}.",
title: "Puma restarted!", image: :success
)
end
else
UI.info "Puma NOT restarted, check your log files."
Compat::UI.info "Puma NOT restarted, check your log files."
if options[:notifications].include?(:not_restarted)
Notifier.notify("Puma NOT restarted, check your log files.", :title => "Puma NOT restarted!", :image => :failed)
Compat::UI.notify(
"Puma NOT restarted, check your log files.",
title: "Puma NOT restarted!", image: :failed
)
end
end
end

def stop
return unless options[:start_on_start]
if options[:notifications].include?(:stopped)
Notifier.notify("Until next time...", :title => "Puma shutting down.", :image => :pending)
Compat::UI.notify(
"Until next time...",
title: "Puma shutting down.", image: :pending
)
end
runner.halt
end
Expand Down
75 changes: 45 additions & 30 deletions lib/guard/puma/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,49 +6,50 @@ class PumaRunner

MAX_WAIT_COUNT = 20

attr_reader :options, :control_url, :control_token, :cmd_opts
attr_reader :options, :control_url, :control_token, :cmd_opts, :pumactl

def initialize(options)
@control_token = options.delete(:control_token) { |_| ::Puma::Configuration.random_token }
@control = "localhost"
@control_port = (options.delete(:control_port) || '9293')
@control_url = "#{@control}:#{@control_port}"
@control_url = "localhost:#{@control_port}"
@quiet = options.delete(:quiet) { true }
@pumactl = options.delete(:pumactl) { false }
@options = options

puma_options = if options[:config]
{
'--config' => options[:config],
'--control-token' => @control_token,
'--control' => "tcp://#{@control_url}",
'--environment' => options[:environment]
}
puma_options = {
(pumactl ? '--config-file' : '--config') => options[:config],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of this syntax. I see what it's doing, but I'd prefer something more straightforward.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made private constant with keys and private method for getting right key. What do you think?

'--control-token' => @control_token,
(pumactl ? '--control-url' : '--control') => "tcp://#{@control_url}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. This is a bit too clever for my tastes 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also changed.

}
if options[:config]
puma_options['--config'] = options[:config]
else
{
'--port' => options[:port],
'--control-token' => @control_token,
'--control' => "tcp://#{@control_url}",
'--environment' => options[:environment]
}
puma_options['--port'] = options[:port]
end
[:bind, :threads].each do |opt|
puma_options["--#{opt}"] = options[opt] if options[opt]
%i[bind threads environment].each do |opt|
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, is there another way these could be checked with out using multiple next's? I find this kind of imperative looping with conditionals to be difficult to reason about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One next I took out into Array#select, but this is double iterating. 🙁

The second next I deleted with else adding.

next unless options[opt]
if pumactl
next Compat::UI.warning(
"`#{opt}` option is not compatible with `pumactl` option"
)
end
puma_options["--#{opt}"] = options[opt]
end
puma_options = puma_options.to_a.flatten
puma_options << '-q' if @quiet
puma_options << '--quiet' if @quiet
@cmd_opts = puma_options.join ' '
end

def start
if in_windows_cmd?
Kernel.system windows_start_cmd
else
Kernel.system nix_start_cmd
end
Kernel.system build_command('start')
end

def halt
Net::HTTP.get build_uri('halt')
if pumactl
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like these can be pushed down a level too so we don't have so many of these conditionals.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you.

Kernel.system build_command('halt')
else
Net::HTTP.get build_uri('halt')
end
# server may not have been stopped correctly, but we are halting so who cares.
return true
end
Expand All @@ -69,7 +70,11 @@ def sleep_time
private

def run_puma_command!(cmd)
Net::HTTP.get build_uri(cmd)
if pumactl
Kernel.system build_command(cmd)
else
Net::HTTP.get build_uri(cmd)
end
return true
rescue Errno::ECONNREFUSED => e
# server may not have been started correctly.
Expand All @@ -80,12 +85,22 @@ def build_uri(cmd)
URI "http://#{control_url}/#{cmd}?token=#{control_token}"
end

def nix_start_cmd
%{sh -c 'cd #{Dir.pwd} && puma #{cmd_opts} &'}
def build_command(cmd)
puma_cmd = "#{pumactl ? 'pumactl' : 'puma'} #{cmd_opts} #{cmd if pumactl}"
background = cmd == 'start'
if in_windows_cmd?
windows_cmd(puma_cmd, background)
else
nix_cmd(puma_cmd, background)
end
end

def nix_cmd(puma_cmd, background = false)
%(sh -c 'cd #{Dir.pwd} && #{puma_cmd} #{'&' if background}')
end

def windows_start_cmd
%{cd "#{Dir.pwd}" && start "" /B cmd /C "puma #{cmd_opts}"}
def windows_cmd(puma_cmd, background = false)
%(cd "#{Dir.pwd}" && #{'start "" /B' if background} cmd /C "#{puma_cmd}")
end

def in_windows_cmd?
Expand Down
Loading