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

Silence a ROR 7.1 deprecation warning by using Rails.cache as the default cache, if its available #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

redterror
Copy link
Contributor

This PR started from an investigation into a deprecation warning on a ROR 7.0 -> 7.1 upgrade. The specific error at the time was:

DEPRECATION WARNING: Support for `config.active_support.cache_format_version = 6.1` has been deprecated and will be removed in Rails 7.2.

Check the Rails upgrade guide at https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#new-activesupport-cache-serialization-format
for more information on how to upgrade.

This is triggered by this line in lib/warden/cognito.rb:

setting :cache, default: ActiveSupport::Cache::NullStore.new

Of course, nowhere here is cache_format_version explicitly set (nor should it be), but rather this is the default value in ActiveSupport.

  • In the non-rails context, the warning is obvious - the default will change in an upcoming release.
  • In a rails context this is confusing because it gets displayed even in the presence of a config.load_defaults 7.1 call in an app's config/application.rb file. I believe this is because the NullStore.new call gets executed at require time and thus before the Rails app config hooks start firing.

Workarounds:

  • We could call ActiveSupport.cache_format_version = XX in warden-cognito's land, if Rails isn't defined. This seems bad because it might mess with an app's settings, and we don't really need to enforce a particular cache format here - that's not our job.
  • We can change the default to be Rails.cache when Rails is defined.
  • Other?

I opted for the 2nd option here since it seemed less intrusive and more in-line with what I'd expect in a Rails context. It also removed the deprecation in my original app.

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

Successfully merging this pull request may close these issues.

1 participant