samvera / questioning_authority

Question your authorities
Other
54 stars 30 forks source link

wrap initialization in reloader.to_prepare to avoid autoload deprecation warning in some cases #349

Closed jrochkind closed 2 years ago

jrochkind commented 2 years ago

My Rails 6.1.3.2 app that uses qa gem has this deprecation warning on startup in development mode:

DEPRECATION WARNING: Initialization autoloaded the constants Qa::LinkedData and Qa::LinkedData::AuthorityService.

Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload Qa::LinkedData, for example,
the expected changes won't be reflected in that stale Module object.

These autoloaded constants have been unloaded.

In order to autoload safely at boot time, please wrap your code in a reloader
callback this way:

    Rails.application.reloader.to_prepare do
      # Autoload classes and modules needed at boot time here.
    end

That block runs when the application boots, and every time there is a reload.
For historical reasons, it may run twice, so it has to be idempotent.

Check the "Autoloading and Reloading Constants" guide to learn more about how
Rails autoloads and reloads.
 (called from <main> at /Users/jrochkind/code/scihist_digicoll/config/environment.rb:5

/Users/jrochkind/code/scihist_digicoll/config/environment.rb:5 is just Rails.application.initialize!.

I haven't been able to reproduce this in a new fresh Rails 6.1.3.2 app -- I'm not sure what settings in my actual app are triggering this warning, when I can't get a fresh app too. I've spent some time trying.

However, it seems like this is a legit problem, that could affect other apps, and should be fixed. Here is the Rails Autoloading and Reloading Constants Guide.

The small change in this PR eliminates the deprecation warning for me. And I believe is appropriate for Rails.

Other possible changes could have been:

Those might actually make sense, but I was nervous to try to make more than the smallest change that seemed to resolve the problem, which this is.

I have also confirmed:

The only thing that makes me nervous is I don't understand why I can't reproduce in stock from scratch app. Still, I think this change should be harmless and appropriate and will fix this problem -- unless someone would prefer we go removing all the extend ActiveSupport::Autoload sprinkled throughout the gem, which may actually be a more fundamental fix.

cjcolvar commented 2 years ago

I'm running into the following error when trying to build Hyrax's docker images.

NoMethodError: undefined method `[]' for nil:NilClass
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:20:in `subauthorities_path'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:29:in `names'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:61:in `register_defaults'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:41:in `block in registry'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local/registry.rb:6:in `initialize'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:40:in `new'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:40:in `registry'
/usr/local/bundle/gems/qa-5.8.0/lib/qa/authorities/local.rb:51:in `register_subauthority'
/app/samvera/hyrax-webapp/config/initializers/hyrax.rb:55:in `<main>'

This is happening when bundle exec rake assets:precompile is run.

I played with different forms and the only I was seeing as working were Qa::Engine.config.before_initialize and the original unwrapped form. This might have to do with the rake command being run in a production rails environment where cache_classes is set to true.

jrochkind commented 2 years ago

Sorry I'm off on Fridays so just going through all my messages and alerts and getting to this.

While we've already reverted the change thinking we didn't have the bandwidth...

For future reference, I think the answer would/could have been that the thing in your local hyrax-webapp/config/initializers/hyrax.rb:55 also needed to be wrapped in to_prepare.