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

Update rack-timeout setter in puma recipe #185

Closed
rjherrera opened this issue Apr 11, 2019 · 2 comments · Fixed by #188
Closed

Update rack-timeout setter in puma recipe #185

rjherrera opened this issue Apr 11, 2019 · 2 comments · Fixed by #188

Comments

@rjherrera
Copy link
Member

In rack_timeout's 0.5.0 release, legacy setters were removed (zombocom/rack-timeout#125), which made this line not longer functional.

In order to fix it, similar to what is suggested in zombocom/rack-timeout#130, it should be changed to:

config.middleware.insert_before Rack::Runtime, Rack::Timeout, service_timeout: (ENV["RACK_TIMEOUT"] || 10).to_i
@blackjid
Copy link
Member

Great catch!

What do you think about using the environment variables support already baked into rack-timeout?

https://github.com/heroku/rack-timeout#configuring

@rjherrera
Copy link
Member Author

I also saw that, and it looks cleaner. It would imply deleting that line, but I'm not sure on how to communicate this, maybe explicitly describing that expected behavior in the README.md?

I know that if we delete the line and the env var is not defined, it would not break, but it feels like hiding something that was explicit before. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants