ankane/slowpoke

Any plan to update with latest rack-timeout?

mintuhouse opened this issue · 9 comments

@ankane thanks for this gem and awesome documentation on ruby timeouts.

This is more of a query than a bug.

The newer version of rack-timeout seems to have changed a lot (looking at the file list) and the API also has changed (timeout is now service_timeout, setting the timeouts via middleware seems to be the preferred option)
I wanted to know if you the issue mentioned in zombocom/rack-timeout#39 still exist? Are there any better ways of handling it today?

Hey @mintuhouse, I haven't tested it with the latest version yet, but from the docs, looks like it should work fine. I'll test when I have some time.

(fwiw, setting timeouts in an initializer still looks like the recommended approach for Rails)

bf4 commented

I just

# per  https://github.com/heroku/rack-timeout/blob/v0.4.2/lib/rack/timeout/core.rb#L66
Rails.application.config
         .middleware.insert_after ActionDispatch::RequestId, Rack::Timeout,
           service_timeout: ENV.fetch('REQUEST_TIMEOUT').to_i,

Did anyone get a chance to test this?

bf4 commented

@mooreds what I wrote is what I use, if you consider that tested. (Though, I actually did write tests for it, IIRC)

@bf4 thanks! Did your change go into lib/slowpoke/railtie.rb ? At this line? https://github.com/ankane/slowpoke/blob/master/lib/slowpoke/railtie.rb#L8

bf4 commented

@mooreds no, that was from an initializer.

I avoided the railtie by specifying in my Gemfile

  gem 'rack-timeout', '~> 0.4', require: 'rack/timeout/base', group: :test

No monkeypatching needed :)

bf4 commented

@mooreds oh, sorry. I didn't end up using slowpoke. But similar idea to changing the require and using your own initializer

bf4 commented

I wonder how this changes with Rails 5 executors https://github.com/rails/rails/pull/27494/files

Just released a new version, which works with the latest version of rack-timeout (the previous version did as well).