Implement AWS as a cache storage #18
Conversation
|
Also I would like to get some input if it's worth to implement copy AND move selected by carrierwave configuration. I don't see any advantage. |
|
@filipegiusti I agree, there doesn't appear to be any benefit to implementing both copy and move. |
| @@ -73,11 +91,23 @@ def size | |||
| end | |||
|
|
|||
| def store(new_file) | |||
| @file = bucket.objects[path].write(uploader_write_options(new_file)) | |||
| if new_file.is_a?(self.class) | |||
| new_file.move_to(path) | |||
There was a problem hiding this comment.
The return value of move_to is self, whereas the return value otherwise is a file object. I'm not sure if this has any repercussions in libraries where people are using the value returned from store.
There was a problem hiding this comment.
The #store return value is always true, not sure why.
As the only documentation was the code, I copied the behavior of https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/storage/fog.rb#L324 and https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/storage/fog.rb#L415 that seems to be good enough.
|
@filipegiusti: It doesn't look like a new CarrierWave version has been released. Still leaving this hanging? |
|
I'm not sure what to do. But I can tell you I've been using this code in production since May. |
|
Perhaps we can pull this next year at the rate CarrierWave releases new versions. I'm going to close it out for now though, just to thin the issue box. |
|
@sorentwo the final of carrierwave 1.0 was released, so I think it would make sense to reconsider this PR. |
|
@fschwahn Thanks for letting me know. Can't believe it has been 2.5 years! @filipegiusti Any chance you want to pick this back up and finish it off? If not, I'll start from what you worked on. |
|
@sorentwo I will try to finish this before carnival, so if you don't hear from me before Saturday, feel free to tackle it. |
|
In fact I already implemented it as I needed it for a project - I can submit a new PR, if @filipegiusti is ok with it? |
|
@fschwahn I'm 😃 with it. Submit it! |
The commits carrierwaveuploader/carrierwave@7a993542701b that's related to carrierwaveuploader/carrierwave#1312 added the option to switch cache storages. carrierwave-aws needs that.
However the carrierwave version with the functionality wasn't published yet, so this implementation isn't ready to be merged, this PR is a WIP to get the code ready for when that's published.