Hi,
This might be a bit silly, but finally I decided to bring this up.
We''re running a Rails app along with another Rack app in
the same config.ru, and what I want to do is telling Unicorn
that we don''t want any middleware which Unicorn might
insert with RACK_ENV=development and RACK_ENVdeployment, because we''re
adding Rack::CommonLogger
and other middleware by ourselves, while we''re using
RACK_ENV=development for Rails when developing.
That is, we want to use RACK_ENV=development for Rails
because that''s how it used to be, but we also don''t want
those additional middleware got inserted automatically.
This has no problem with RACK_ENV=production.
I know this is somehow a spec from Rack, but I guess I
don''t like this behaviour. One workaround would be
providing a `after_app_load'' hook, and we add this to
the bottom of config.ru:
ENV[''RACK_ENV_ORIGINAL''] = ENV[''RACK_ENV'']
ENV[''RACK_ENV''] = ''none''
and add this to the unicorn configuration file:
after_app_load do |_|
ENV[''RACK_ENV''] =
ENV[''RACK_ENV_ORIGINAL'']
end
This is probably a stupid hack, but I wonder if after_app_load
hook would be useful for other cases?
Or if we could have an option to turn off this Rack behaviour
simulation, like:
default_middleware false
That might be more straightforward for what we want. Oh but
it seems it''s not that easy to add this. What about an option
for unicorn?
unicorn -E development -N
or
unicorn -E development --no-default-middleware
This would need to be applied to `rainbows'' and `zbatery'',
too,
though. Patches below for consideration:
(Sorry if Gmail messed the format up, but from last time
I tried, it doesn''t accept attachments :( What''s the best way?
Links to raw patches?)
https://github.com/godfat/unicorn/pull/2
commit 95de5abf38a81a76af15476d4705713d2d644664
Author: Lin Jen-Shin <godfat at godfat.org>
Date: Fri Jan 25 18:18:21 2013 +0800
Add `after_app_load'' hook.
The hook would be called right after application is loaded.
diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 7651093..332bdbc 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -43,6 +43,9 @@ class Unicorn::Configurator
:before_exec => lambda { |server|
server.logger.info("forked child re-executing...")
},
+ :after_app_load => lambda { |server|
+ server.logger.info("application loaded")
+ },
:pid => nil,
:preload_app => false,
:check_client_connection => false,
@@ -171,6 +174,13 @@ class Unicorn::Configurator
set_hook(:before_exec, block_given? ? block : args[0], 1)
end
+ # sets the after_app_load hook to a given Proc object. This
+ # Proc object will be called by the master process right
+ # after application loaded.
+ def after_app_load(*args, &block)
+ set_hook(:after_app_load, block_given? ? block : args[0], 1)
+ end
+
# sets the timeout of worker processes to +seconds+. Workers
# handling the request/app.call/response cycle taking longer than
# this time period will be forcibly killed (via SIGKILL). This
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index aa98aeb..a3b30ee 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -14,6 +14,7 @@ class Unicorn::HttpServer
# :stopdoc:
attr_accessor :app, :request, :timeout, :worker_processes,
:before_fork, :after_fork, :before_exec,
+ :after_app_load,
:listener_opts, :preload_app,
:reexec_pid, :orig_app, :init_listeners,
:master_pid, :config, :ready_pipe, :user
@@ -716,6 +717,7 @@ class Unicorn::HttpServer
Gem.refresh
end
self.app = app.call
+ config.after_app_load.call(self)
end
end
And --no-default-middleware
https://github.com/godfat/unicorn/pull/3
commit e3575db2a36e3ca2acda18bfee97bf95609a9860
Author: Lin Jen-Shin <godfat at godfat.org>
Date: Fri Jan 25 18:38:52 2013 +0800
Add -N or --no-default-middleware option.
This would prevent Unicorn from adding default middleware,
as if RACK_ENV is always none. (not development nor deployment)
This should also apply to `rainbows'' and `zbatery''.
diff --git a/bin/unicorn b/bin/unicorn
index 9962b58..415d164 100755
--- a/bin/unicorn
+++ b/bin/unicorn
@@ -58,6 +58,11 @@ op = OptionParser.new("", 24, ''
'') do |opts|
ENV["RACK_ENV"] = e
end
+ opts.on("-N", "--no-default-middleware",
+ "no default middleware even if RACK_ENV is development") do
|e|
+ rackup_opts[:no_default_middleware] = true
+ end
+
opts.on("-D", "--daemonize", "run daemonized in the
background") do |d|
rackup_opts[:daemonize] = !!d
end
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index d96ff91..f0ceffe 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -49,6 +49,8 @@ module Unicorn
pp({ :inner_app => inner_app }) if $DEBUG
+ return inner_app if op[:no_default_middleware]
+
# return value, matches rackup defaults based on env
# Unicorn does not support persistent connections, but Rainbows!
# and Zbatery both do. Users accustomed to the Rack::Server default
"Lin Jen-Shin (godfat)" <godfat at godfat.org> wrote:> Hi, > > This might be a bit silly, but finally I decided to bring this up. > > We''re running a Rails app along with another Rack app in > the same config.ru, and what I want to do is telling Unicorn > that we don''t want any middleware which Unicorn might > insert with RACK_ENV=development and RACK_ENV> deployment, because we''re adding Rack::CommonLogger > and other middleware by ourselves, while we''re using > RACK_ENV=development for Rails when developing.Doesn''t Rails favor RAILS_ENV over RACK_ENV? unicorn ignores RAILS_ENV (unicorn_rails respects RAILS_ENV, but unicorn_rails isn''t recommended for modern (Rack-enabled) Rails)> That is, we want to use RACK_ENV=development for Rails > because that''s how it used to be, but we also don''t want > those additional middleware got inserted automatically. > This has no problem with RACK_ENV=production.Is there a benefit which RACK_ENV=development gives you for Rails that RAILS_ENV=development does not give you? Fwiw, the times I worked on Rails, I used customized dev environments anyways, so I would use RAILS_ENV=dev_local or RAILS_ENV=dev_$NETWORK for switching between a localhost-only vs networked setup.> I know this is somehow a spec from Rack, but I guess I > don''t like this behaviour. One workaround would be > providing a `after_app_load'' hook, and we add this to > the bottom of config.ru: > > ENV[''RACK_ENV_ORIGINAL''] = ENV[''RACK_ENV''] > ENV[''RACK_ENV''] = ''none'' > > and add this to the unicorn configuration file: > > after_app_load do |_| > ENV[''RACK_ENV''] = ENV[''RACK_ENV_ORIGINAL''] > end > > This is probably a stupid hack, but I wonder if after_app_load > hook would be useful for other cases?I don''t see how this hook has benefits over putting this in config.ru (or somewhere else in your app, since your app is already loaded...) I''d imagine users wanting the same app-wide settings going between different application servers, too.> Or if we could have an option to turn off this Rack behaviour > simulation, like: > > default_middleware false > > That might be more straightforward for what we want. Oh but > it seems it''s not that easy to add this. What about an option > for unicorn? > > unicorn -E development -N > > or > > unicorn -E development --no-default-middlewareI''m against adding more command-line or config options, especially around loading middleware. RACK_ENV is already a source of confusion, and Rails also has/(or had?) it''s own middleware loading scheme independent of config.ru.> This would need to be applied to `rainbows'' and `zbatery'', too, > though. Patches below for consideration: > (Sorry if Gmail messed the format up, but from last time > I tried, it doesn''t accept attachments :( What''s the best way? > Links to raw patches?)git format-patch + git send-email is the recommended way if your mail client cannot be taught to leave formatting alone. Also this is easy for sending multiple patches in the same thread. git format-patch --stdout with a good mail client which will not wrap messages is also great (especially for single patches). Attachments (generated using git format-patch) for text/x-diff should be accepted by Mailman. Attached patches are a last resort, I prefer inline patches since I can quote/reply to them inline. (same patch submission guidelines as git or Linux kernel)> https://github.com/godfat/unicorn/pull/2 > > commit 95de5abf38a81a76af15476d4705713d2d644664 > Author: Lin Jen-Shin <godfat at godfat.org> > Date: Fri Jan 25 18:18:21 2013 +0800 > > Add `after_app_load'' hook. > > The hook would be called right after application is loaded.Commit messages should explain why a change is made/needed, not just what it does.
On Sat, Jan 26, 2013 at 2:39 AM, Eric Wong <normalperson at yhbt.net> wrote:> Doesn''t Rails favor RAILS_ENV over RACK_ENV? unicorn ignores RAILS_ENV > (unicorn_rails respects RAILS_ENV, but unicorn_rails isn''t recommended > for modern (Rack-enabled) Rails)Yes, it is. But it would still look into RACK_ENV if RAILS_ENV is not available. One solution would be setting RAILS_ENV=development while setting RACK_ENV=none. I don''t really want to do this for several reasons: * It would be better to keep RAILS_ENV == RACK_ENV to avoid confusion. * It might be tricky to make RAILS_ENV default to development while setting RACK_ENV since Rails would be looking into it. So we need to make sure that Rails is loaded and default to development and then we can setup RACK_ENV to avoid messing up the original Rails default.> Is there a benefit which RACK_ENV=development gives you for Rails > that RAILS_ENV=development does not give you?The main reason is that I want to avoid confusion. The other reason is that that''s also how Heroku tries to use. I think it''s also because of consistency. (so that we don''t have to distinguish RACK_ENV and RAILS_ENV) The other reason would be making them different might cause some issues as Rails would also be looking at RACK_ENV.> Fwiw, the times I worked on Rails, I used customized dev environments > anyways, so I would use RAILS_ENV=dev_local or RAILS_ENV=dev_$NETWORK > for switching between a localhost-only vs networked setup.I would also do this I guess, but since I am not the only one working on the application, I would avoid making such changes. We have a lot of configurations depending on the word "development" already.>> I know this is somehow a spec from Rack, but I guess I >> don''t like this behaviour. One workaround would be >> providing a `after_app_load'' hook, and we add this to >> the bottom of config.ru: >> >> ENV[''RACK_ENV_ORIGINAL''] = ENV[''RACK_ENV''] >> ENV[''RACK_ENV''] = ''none'' >> >> and add this to the unicorn configuration file: >> >> after_app_load do |_| >> ENV[''RACK_ENV''] = ENV[''RACK_ENV_ORIGINAL''] >> end >> >> This is probably a stupid hack, but I wonder if after_app_load >> hook would be useful for other cases? > > I don''t see how this hook has benefits over putting this in config.ru > (or somewhere else in your app, since your app is already loaded...)It''s better to me because it''s clear on the loading order. The point where to change RACK_ENV=none would be after Rails loaded (to avoid messing Rails up), and _before Unicorn inserting middleware_. And then I would like to switch RACK_ENV=RAILS_ENV to avoid confusion. It''s hard to tell where I can put this code.> I''d imagine users wanting the same app-wide settings going between > different application servers, too.Speaking to this, I might want to complain about the other severs :P As far as I know, only if you use `rackup'' would you get those middleware. That is, `unicorn'' is compatible with `rackup''. That''s a good thing. But it seems to me that both `thin'' and `puma'' (and maybe including others, I didn''t check) would not do this if you use their command line tools. They even have different options to enable Rack::CommonLogger. Actually this is also the reason why sometimes `thin'' and `unicorn'' worked differently when people switching from Thin to Unicorn. And you know a lot of people are using Thin in the first place, thus sometimes blaming Unicorn works differently. I am not sure how many people are using `rackup'', but according to my observation, not too much. If people are using Sinatra directly, eventually they would launch the server via Rack::Handler.get.run, which does not try to wrap additional middleware as what `rackup'' would do if I read correctly. So I am not sure if a lot of people would be expecting this actually. I used to use `rackup'' a lot to get consistent behaviour, but it looks to me not many people even know the existence of rackup :(> I''m against adding more command-line or config options, especially > around loading middleware. RACK_ENV is already a source of confusion, > and Rails also has/(or had?) it''s own middleware loading scheme > independent of config.ru.Yes, I hate that, too. I can''t complain about Rails less :P I don''t understand why people are building those middleware scheme on their own way, even Sinatra does this. I guess they want more control over middleware, so that they can reorder, delete, insert more/less middleware somehow. I guess that makes sense in some way... but they should be really in Rack instead of each web framework IMHO.> git format-patch + git send-email is the recommended way if your > mail client cannot be taught to leave formatting alone. Also this > is easy for sending multiple patches in the same thread. > > git format-patch --stdout with a good mail client which will not wrap > messages is also great (especially for single patches). > > Attachments (generated using git format-patch) for text/x-diff should be > accepted by Mailman. Attached patches are a last resort, I prefer > inline patches since I can quote/reply to them inline. > > (same patch submission guidelines as git or Linux kernel)Last time I tried to setup SMTP for git send-email failed. I''ll try again next time. Thanks!> Commit messages should explain why a change is made/needed, not just > what it does.Yeah, I know, but I can understand if none of them is acceptable, (that''s why I didn''t bring this up in the first place) and the reason to build those features might be arguing. So instead of writing them carefully, I decided to bring this up sooner and then we can work out the true reason later on :D Thanks a lot for your time.
"Lin Jen-Shin (godfat)" <godfat at godfat.org> wrote:> On Sat, Jan 26, 2013 at 2:39 AM, Eric Wong <normalperson at yhbt.net> wrote: > > Doesn''t Rails favor RAILS_ENV over RACK_ENV? unicorn ignores RAILS_ENV > > (unicorn_rails respects RAILS_ENV, but unicorn_rails isn''t recommended > > for modern (Rack-enabled) Rails) > > Yes, it is. But it would still look into RACK_ENV if RAILS_ENV is not available. > One solution would be setting RAILS_ENV=development while setting > RACK_ENV=none. I don''t really want to do this for several reasons: > > * It would be better to keep RAILS_ENV == RACK_ENV to avoid confusion. > * It might be tricky to make RAILS_ENV default to development while setting > RACK_ENV since Rails would be looking into it. So we need to make sure > that Rails is loaded and default to development and then we can setup > RACK_ENV to avoid messing up the original Rails default.OK, I agree making RAILS_ENV==RACK_ENV in deployments is less confusing. (unicorn should continue NOT enforcing this internally, though)> > I''d imagine users wanting the same app-wide settings going between > > different application servers, too. > > Speaking to this, I might want to complain about the other severs :P > As far as I know, only if you use `rackup'' would you get those middleware. > > That is, `unicorn'' is compatible with `rackup''. That''s a good thing. > But it seems to me that both `thin'' and `puma'' (and maybe including > others, I didn''t check) would not do this if you use their command line > tools. They even have different options to enable Rack::CommonLogger. > > Actually this is also the reason why sometimes `thin'' and `unicorn'' > worked differently when people switching from Thin to Unicorn. > And you know a lot of people are using Thin in the first place, > thus sometimes blaming Unicorn works differently. > > I am not sure how many people are using `rackup'', but according to > my observation, not too much. If people are using Sinatra directly, > eventually they would launch the server via Rack::Handler.get.run, > which does not try to wrap additional middleware as what `rackup'' > would do if I read correctly. > > So I am not sure if a lot of people would be expecting this actually. > I used to use `rackup'' a lot to get consistent behaviour, but it looks to > me not many people even know the existence of rackup :(*sigh* :< I kinda wish there was no default middleware, either. But it''s too late to change unicorn defaults. I think your -N/--no-default-middleware patch can be accepted, then. after_app_load is more prone to user error/confusion, I think (I believe RAILS_ENV/RACK_ENV should be frozen for the lifetime of the process)> Last time I tried to setup SMTP for git send-email failed. > I''ll try again next time. Thanks!Can you try send-email again with the --no-default-middleware patch? :) The git-send-email(1) manpage has several good examples for your email provider. There''s also git-imap-send which you can use to stick email in your Drafts folder, but I''ve never needed/used it, though.
On Tue, Jan 29, 2013 at 7:21 AM, Eric Wong <normalperson at yhbt.net> wrote:> "Lin Jen-Shin (godfat)" <godfat at godfat.org> wrote: >> Speaking to this, I might want to complain about the other severs :P >> As far as I know, only if you use `rackup'' would you get those middleware. >> >> That is, `unicorn'' is compatible with `rackup''. That''s a good thing. >> But it seems to me that both `thin'' and `puma'' (and maybe including >> others, I didn''t check) would not do this if you use their command line >> tools. They even have different options to enable Rack::CommonLogger. >> >> Actually this is also the reason why sometimes `thin'' and `unicorn'' >> worked differently when people switching from Thin to Unicorn. >> And you know a lot of people are using Thin in the first place, >> thus sometimes blaming Unicorn works differently. >> >> I am not sure how many people are using `rackup'', but according to >> my observation, not too much. If people are using Sinatra directly, >> eventually they would launch the server via Rack::Handler.get.run, >> which does not try to wrap additional middleware as what `rackup'' >> would do if I read correctly. >> >> So I am not sure if a lot of people would be expecting this actually. >> I used to use `rackup'' a lot to get consistent behaviour, but it looks to >> me not many people even know the existence of rackup :( > > *sigh* :< > > I kinda wish there was no default middleware, either. But it''s > too late to change unicorn defaults. > > I think your -N/--no-default-middleware patch can be accepted, > then. after_app_load is more prone to user error/confusion, I think > (I believe RAILS_ENV/RACK_ENV should be frozen for the lifetime of > the process)Awesome. Thanks! Hope this would also be helpful for others.> Can you try send-email again with the --no-default-middleware patch? :) > > The git-send-email(1) manpage has several good examples for your email > provider. There''s also git-imap-send which you can use to stick email > in your Drafts folder, but I''ve never needed/used it, though.Just got managed to send a test mail to myself. I realized that my git is using /usr/bin/perl instead of /usr/local/bin/perl, so that instead of running `cpan Net::SMTP::SSL` I have to run `sudo -H /usr/bin/cpan Net::SMTP::SSL` following instructions from this site: http://kbase.wincent.com/old/knowledge-base/Installing_Net::SMTP::SSL_for_sending_patches_with_Git_over_secure_SMTP.html I''ll send the patch shortly after updating the commit message. Feel free to edit it as usual :) And should I send patches for rainbows and zbatery as well?