Allow providing upload options to attacher.create_derivatives

I have FileSystem cache storage and S3 store.
I need to process derivatives and put them to cache. Later backgrounding will upload it to S3.
But attacher.create_derivatives always uploads files to store storage.

attacher.add_derivatives method accepts :storage argument, but there are no way to pass it through attacher.create_derivatives

I have workaround by changing create_derivatives from to

    def create_derivatives(*args, **opts)
      files = process_derivatives(*args, **opts)
      add_derivatives(files, **opts)
    end

Yeah, with Attacher#create_derivatives I had to decide whether options will be forwarded to Attacher#process_derivatives or Attacher#add_derivatives (ultimately Attacher#upload_derivatives). I didn’t want to forward it to both, as “upload” options would be irrelevant for the processor, and “process” options would be irrelevant for the uploader, so it could be confusing (even though it would still technically work, as the uploader would ignore any unrecognized options).

So, I thought that if someone wants to pass upload options, they can drop down a level lower and use Attacher#process_derivatives + Attacher#add_derivatives directly. However, I think that :storage is a common enough option that we can add it to Attacher#create_derivatives. And that option is actually different from other uploader options (such as :upload_options, :metadata etc), for which it’s still reasonable to allow only through the lower level API.

I will then add :storage to Attacher#create_derivatives shortly :slight_smile:

By the way, don’t you think that it would be clearer by default to use cache storage for derivatives?

Its weird that with backgrounding plugin this code places original file to cache storage and derivations to permanent store.

    image = Image.create!(file: ...), 
    image.file_derivatives!
    image.save!

How would the derivatives plugin know for what purpose you’re generating derivatives for? E.g. if you call Attacher#create_derivatives in the controller versus in a background job, how would the derivatives plugin know the difference?

In your example, why do you want derivatives to be uploaded to temporary storage, if the record is already assumed to be valid? Even if you want to generate some derivatives “eagerly” (in the controller instead of a background job), I think people will most often want to upload them directly to permanent storage.

Look at explanation of backgrounding plugin:

The backgrounding plugin enables you to move promoting and deleting of files into background jobs. This is especially useful if you’re processing derivatives and storing files to a remote storage service.

So when users uploads images (direct upload to server), I’m processing derivatives, store them in public folder on same server (FileSystem cache storage), and generate response to user with processed files links.

I assume it will be better to avoid external http calls inside db transaction.
Later background job will upload this derivatives to S3.
So I’m saving request time for my users. Also I’m avoiding troubles with http errors, as backgrounding job will restart.


How would the derivatives plugin know for what purpose you’re generating derivatives for?

For me, it seems right that default derivatives storage must be same where original file is located.

I assume it will be better to avoid external http calls inside db transaction.
Later background job will upload this derivatives to S3.
So I’m saving request time for my users. Also I’m avoiding troubles with http errors, as backgrounding job will restart.

Ok, yeah, that makes sense :+1:

For me, it seems right that default derivatives storage must be same where original file is located.

If this was the case, then in the recommended example for creating derivatives in the background, the derivatives would first get uploaded to temporary storage, then to permanent on promotion, which would create extra overhead.

class PromoteJob
  def perform(...)
    attacher = attacher_class.retrieve(...)
    # Here the original file is still in cache storage, so
    # the derivatives would get uploaded to cache storage
    # as well.
    attacher.create_derivatives
    # Then here derivatives would get uploaded to permanent
    # storage.
    attacher.atomic_promote
  end
end

I don’t think it would make sense to match derivatives storage to the current original file storage by default, as the original file would be uploaded to temporary storage for separate reasons not necessarily related to derivatives (direct uploads or retaining across form redisplays).

Also, it’s most common to want to upload derivatives straight to permanent storage, in which case users would need to override :storage to get that behaviour (and they couldn’t use the global :storage plugin option for that).

Note that you can override derivatives storage settings to get the behaviour you want:

class MyUploader < Shrine
  # use original file storage as the default derivatives storage
  Attacher.derivatives_storage { file.storage_key }
end

Thanks a million for this super explanation :+1: