Skip to content

Commit

Permalink
Final blow to CGI
Browse files Browse the repository at this point in the history
  • Loading branch information
josh committed Apr 14, 2009
1 parent 11d4bfb commit 4a3afe0
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 268 deletions.
15 changes: 0 additions & 15 deletions actionpack/lib/action_controller/cgi/ext.rb

This file was deleted.

112 changes: 0 additions & 112 deletions actionpack/lib/action_controller/cgi/ext/cookie.rb

This file was deleted.

22 changes: 0 additions & 22 deletions actionpack/lib/action_controller/cgi/ext/query_extension.rb

This file was deleted.

24 changes: 0 additions & 24 deletions actionpack/lib/action_controller/cgi/ext/stdinput.rb

This file was deleted.

75 changes: 0 additions & 75 deletions actionpack/lib/action_controller/cgi/process.rb

This file was deleted.

22 changes: 2 additions & 20 deletions actionpack/lib/action_controller/dispatch/dispatcher.rb
Expand Up @@ -23,11 +23,6 @@ def define_dispatcher_callbacks(cache_classes)
end
end

# DEPRECATE: Remove CGI support
def dispatch(cgi = nil, session_options = CgiRequest::DEFAULT_SESSION_OPTIONS, output = $stdout)
new(output).dispatch_cgi(cgi, session_options)
end

# Add a preparation callback. Preparation callbacks are run before every
# request in development mode, and before the first request in production
# mode.
Expand All @@ -43,13 +38,7 @@ def to_prepare(identifier = nil, &block)
end

def run_prepare_callbacks
if defined?(Rails) && Rails.logger
logger = Rails.logger
else
logger = Logger.new($stderr)
end

new(logger).send :run_callbacks, :prepare_dispatch
new.send :run_callbacks, :prepare_dispatch
end

def reload_application
Expand All @@ -76,9 +65,7 @@ def cleanup_application
include ActiveSupport::Callbacks
define_callbacks :prepare_dispatch, :before_dispatch, :after_dispatch

# DEPRECATE: Remove arguments, since they are only used by CGI
def initialize(output = $stdout, request = nil, response = nil)
@output = output
def initialize
@app = @@middleware.build(lambda { |env| self.dup._call(env) })
end

Expand All @@ -97,11 +84,6 @@ def dispatch
end
end

# DEPRECATE: Remove CGI support
def dispatch_cgi(cgi, session_options)
CGIHandler.dispatch_cgi(self, cgi, @output)
end

def call(env)
@app.call(env)
end
Expand Down

8 comments on commit 4a3afe0

@knzconnor
Copy link
Contributor

Choose a reason for hiding this comment

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

Still could use some notes in the documentation on how this effects stock mongrel, mongrel_cluster, thin on most existing deployments, as they call into the old function. If instead, everything should be started through rackup to use the newer adapter/handelers, as ezra suggested to me, maybe that should be noted somewhere more prominently. I know it’s be nice when mongrel_rails et al get updated, but….

@josh
Copy link
Contributor Author

@josh josh commented on 4a3afe0 Apr 17, 2009

Choose a reason for hiding this comment

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

According to Evan Weaver, mongrel_rails is going be removed from Mongrel 2.0. Mongrel will become a simple lib w/ no binaries. Rackup will be the preferred command for booting up your server.

We’ll post something to the Rails blog once this is all settled.

@macournoyer
Copy link

Choose a reason for hiding this comment

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

Maybe add a Rack adapter for Rails in rack core. Then I’ll use this one in Thin.

@knzconnor
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, thanks Josh.

It’s just that since it’s in the major stable release (2.3.2) it can cause a bit of confusion for people not knowing that. We solved it via switching over to rackup, but I am just trying to ease things for everyone else who follows stable releases without necessarily knowing the undocumented expectations, especially deploy-wise.

@josh
Copy link
Contributor Author

@josh josh commented on 4a3afe0 Apr 17, 2009

Choose a reason for hiding this comment

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

@timocratic mongrel_rails should work on 2.3, we have a cgi proxy in place. Accepting patches for any bugs :)

@macournoyer Yeah, maybe I’ll push up a Rack::Adapter::Rails that everyone can count one to boot up Rails. Are you planning on still supporting your “thin” binary? If so what’s your motivation?

@macournoyer
Copy link

Choose a reason for hiding this comment

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

I don’t know yet. That’s kind of a big change, it would break most deployment scripts out there. Thin has builtin support for cluster management which rackup doesn’t have.
I’ll see what Mongrel does and try to synx w/ them. But I’m all for removing code from Thin, so suggestions welcome :)

@knzconnor
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I know it works. ;) It just causes mongrel/thin to go into cgi mode, which can cause some slowdowns if you have metals. The only solutions I see are document the official “use rackup to startup of mongrel_rails/thin/etc” or the workaround maybe the one you suggest.

The main problem with even the document using rackup approach that is exactly what macournoyer points out. Some of the existing start-up methods have better cluster management built in, so it’d be nice if there were a more obvious upgrade path. For use our mongrel_cluster setup is way more robust than rackup for that, but it’d be nice to have the change from 2.2 to 2.3 to not force the app into a deprecated mode that runs slower.

I’m open to suggestions, and would be willing to hack up a patch, if I had a better idea of the best solution.

@josh
Copy link
Contributor Author

@josh josh commented on 4a3afe0 Apr 18, 2009

Choose a reason for hiding this comment

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

@macournoyer It would be awesome if “thin” == “rackup -s thin”. But yeah, rackup is lacking in features. I hear someone is working on porting some of those over. It would be super handy if rackup had all those awesome features.

@timocratic Lets get that CGI problem fixed in 2.3. We are stating 2.3 is still fully CGI compat so people shouldn’t have to switch from mongrel_rails.

Please sign in to comment.