Purple-Devs / health_check

Simple health check of Rails app for use with uptime checking sites like newrelic and pingdom
MIT License
477 stars 127 forks source link

Move controller to app/controllers/health_check #110

Open Crammaman opened 3 years ago

Crammaman commented 3 years ago

The rails convention for a rails engine is to have the folder structure mimic a rails applications. With rails bringing in Zeitwerk this convention is being enforced. By not having the controller under app/controllers the controller (and so the constant ActionController::Base) is getting required during initialization.

This is currently raising the deprecation warning: "Initialization autoloaded the constants ActionText::ContentHelper and ActionText::TagHelper. Being able to do this is deprecated. Autoloading during initialization is going to be an error condition in future versions of Rails."

Moving the controller into app/controllers/health_check and removing the require allows rails to autoload the controller correctly and so silences this deprecation warning.

thedarkside commented 2 years ago

Can we get this merged?

evolve2k commented 2 years ago

Minor change fixes deprecation warning. Looks good to merge :)

jmarchello commented 1 year ago

I'd like to test this a bit before accepting it. It's sat for a couple years now and Rails 7 introduced some changes to autoloading.

Crammaman commented 1 year ago

@jmarchello Fair enough, I can only really confirm that it works in the Rails 6 application I work on.

valscion commented 1 year ago

This change is also needed for Rails to use any config.action_controller values set inside an initializer.

Without this change, health_check gem loads ActionController::Base class before any initializers inside config/initializers/ run. When ActionController::Base is loaded, it copies the configuration values set in Rails.application.config.action_controller.

That is why the outcome of bin/rails app:update during a Rails upgrade process, which creates a config/initializers/*new_framework_defaults.rb file, doesn't work when health_check gem is present (without this PR). More information of this topic over at the Rails issue tracker:

pkordel commented 1 year ago

Bump... please?

valscion commented 1 year ago

For what it's worth, we've been able to work around this issue by setting gem 'health_check', require: false in Gemfile and loading the health_check gem only after ActionController has been loaded via an initializer like this:

# config/initializers/health_check.rb
ActiveSupport.on_load(:action_controller) do
  require 'health_check'
  HealthCheck.setup do |config|
    # ... configuration here ...
  end
end
pkordel commented 1 year ago

Thanks for this information, very helpful!

On Thu, 10 Aug 2023 at 13:29 Vesa Laakso @.***> wrote:

For what it's worth, we've been able to work around this issue by setting gem 'health_check', require: false in Gemfile and loading the health_check gem only after ActionController has been loaded via an initializer like this:

config/initializers/health_checkActiveSupport.on_load(:action_controller) do

require 'health_check' HealthCheck.setup do |config|

... configuration here ...

endend

— Reply to this email directly, view it on GitHub https://github.com/Purple-Devs/health_check/pull/110#issuecomment-1672631421, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABNDXIYSCTFH4VQHA6AITXUR5THANCNFSM46YOWNCA . You are receiving this because you commented.Message ID: @.***>