CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 22k
Restore Active Storage config to disable variants and analyzers #55303
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
Conversation
a82e3f6
to
d7e8540
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pretty much forgot about this already, thanks for checking it out again. :disabled
is much nicer than relying on nil
for the processor.
8300184
to
f1193b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm that this PR restores the ability to prepend custom analyzers to ActiveStorage.analyzers
π
f1193b6
to
118e051
Compare
118e051
to
4d21e4f
Compare
3f97874
to
7364ed9
Compare
5ab955b
to
5fc756c
Compare
Fixes a couple of mistakes made in e978ef0. * Allow setting `config.active_storage.analyzers = []` * Allow disabling `config.active_storage.variant_processor` to remove warning * Ensure original default `ActiveStorage.analyzers` array is not broken (all available analyzers) * Ensure original default `ActiveStorage.variant_processor` is not broken (`:mini_magick` until `load_defaults(7.0)`) This change restores the configuration of analyzers back to 8.0, instead of trying to gracefully handle loading those constants if the associated gem is missing. Due to that config being set at require time, this happens prior to initialization and we don't have access to a logger or deprecator to warn the user. Since `image_processing` gem transitively depends on `ruby-vips` and `mini_magick`, you only need to install that to remove the warning. However, if you're not using variant transformers, but still using the default analyzers you can remove the warning by including those gems or setting the analyzers config to an empty array. We determine the variant transformer based on `config.active_storage.variant_processor` value, or nothing if you passed an invalid value (like `mmmmmini_magick`). Passing an invalid value, should have the same existing behavior and not impact a working application but in the future we'd like to deprecate it. Co-authored-by: Alexandre Ruban <alexandre@hey.com>
5fc756c
to
367445d
Compare
Something isn't quite right I think. I generated a new rails app with $ bin/rails c
/home/user/code/rails/railties/lib/rails/application/configuration.rb:178:in 'Rails::Application::Configuration#load_defaults': undefined method 'analysis=' for nil (NoMethodError)
active_storage.queues.analysis = :active_storage_analysis
^^^^^^^^^^^ bisect lead me back to this. Can someone confirm? |
Oh, this happens when libvips is not installed. I forgot I uninstalled this to test something else. |
FYI my daily-run automated test broke 2 days ago with the same error as @Earlopain. This is an end-to-end test that generates a Rails app with
My GH action uses the I.e. maybe users expect |
Thank you both for checking. I can reproduce after uninstalling libvips with this:
Opening a PR. |
Fixes a couple of mistakes made in e978ef0 (#45100).
config.active_storage.analyzers = []
config.active_storage.variant_processor
to remove warningActiveStorage.analyzers
array is not broken (all available analyzers)ActiveStorage.variant_processor
is not broken (:mini_magick
untilload_defaults(7.0)
)This change restores the configuration of analyzers back to 8.0, instead of trying to gracefully handle loading those constants if the associated gem is missing.
Due to that config being set at require time, this happens prior to initialization and we don't have access to a logger or deprecator to warn the user.
Since
image_processing
gem transitively depends onruby-vips
andmini_magick
, you only need to install that to remove the warning.However, if you're not using variant transformers, but still using the default analyzers you can remove the warning by including those gems or setting the analyzers config to an empty array.
We determine the variant transformer based on
config.active_storage.variant_processor
value, or nothing if you passed an invalid value (likemmmmmini_magick
).Passing an invalid value, should have the same existing behavior and not impact a working application but in the future we'd like to deprecate it.
Fixes: #54391
Alternative to: #54402
Fixes: #54152
Alternative to: #54166
/cc @alexandreruban @Earlopain @Edouard-chin @skipkayhil