-
Notifications
You must be signed in to change notification settings - Fork 402
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
Use cache! call to Carrierwave so that base image process functions run in addition to version process calls. #321
base: master
Are you sure you want to change the base?
Conversation
This is to see if base image processing works again.
Bump for this please. |
I've tested this branch against my app and it works, but I needed to make this small change:
Does that make sense? Sorry I don't have much context here, just mashing keys together to get our tests green atm. Thanks! |
So I'm not the maintainer and I was just trying to get this PR up for the issue re: If I'm not getting the issue just let me know. I'm not an expert in this gem, we just saw the original issue with base |
Hi there, I'm opening a PR for an issue we saw recently with an upgrade we made to carrierwave v3 and carrierwave_backgrounder. I opened an issue for it but it has been sitting for a little while (#320 ), so I'll open a PR for the changes. Most of the comments are going to be copied to here.
Before our upgrade:
carrierwave 1.3.2
carrierwave_backgrounder 0.4.3
After:
carrierwave 3.0.5
carrierwave_backgrounder 1.0.2
Amongst many other things that were an issue with carrierwave v3, we noticed that base image processing in a background worker was not working after updating. For example, using an uploader class similar to this:
Excuse the different process functions here for auto rotate. In our code they are the same but it helped with debug logging here. There is a StripExif one as well in the included module but it's basically just another process function. BaseImageUploader is just a class that inherits from ApplicationUploader::Base while specifying a content_type_allowlist. When Carrierwave is used by itself without backgrounder or prior to version 3/backgrounder 1.0.2, the base processing of the resize to 2048x1024 and auto_rotate ran fine. After upgrading, these base image processors no longer run.
I think this is a change in recent Carrierwave versions that recreate_versions! does not apply processing to the base image. We've been using a fork for us of carrierwave_backgrounder where we have switched lib/backgrounder/workers/process_asset_mixin.rb to use
cache!
instead which seems to be both processing the base image as well as versions.It may be a bit clearer to see with some logging. I enable MiniMagick debug logging with an initializer that has
MiniMagick.logger.level = Logger::DEBUG if Rails.env.development?
in it. With the current carrierwave_backgrounder 1.0.2, the logs look like this:QuieterProcessAssetWorker by the way is a worker class of ours that inherits from CarrierWave::Workers::ProcessAsset and has some error catching, no real changes beyond that. So in this log, note that "AUTO ROTATE BASE" is not called/shown and the resize to 2048,1024 does not happen.
Here is a log with our fork/branch where the base resizing/auto rotate occurs as it should:
Note that "AUTO ROTATE BASE" is in the log and the base image resizing is done as expected.
I should say that we primarily use remote_X_url to start the process here. We don't use direct assignment via assigning a file to the mounted uploader, but I just tested it and it is the same situation/issue/fix as above. When I was debugging carrierwave 3.0.5, the call stack that finally arrived at the image processor functions came through
download!
and thencache!
in carrierwave, so even though it looks a bit odd to me to call cache! to process everything, it was the only way I saw to have both the base image processing done as well as the versions.