Aws s3 #2226
Aws s3 #2226
Conversation
|
If we implement this:
|
|
Getting the existing logos might be tricky. If I try to I get a 522 error |
|
Screenshot, just to show it's working: |
jonodrew
force-pushed
the
aws-s3
branch
2 times, most recently
from
804fea0 to
648635d
Compare
|
Remove the FTP gem |
app/uploaders/avatar_uploader.rb
Outdated
| # Override the directory where uploaded files will be stored. | ||
| # This is a sensible default for uploaders that are meant to be mounted: | ||
| def store_dir | ||
| "uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}" | ||
| "uploads/#{model.class.to_s.underscore}/#{model.id}" |
There was a problem hiding this comment.
@jonodrew I don't really mind either way but any reason we removed the mounted_as from the directory?
There was a problem hiding this comment.
I wasn't sure whether it was something unique to SFTP, and the examples I found didn't have it. I'm also indifferent and happy to leave it in - I suspect it'll just end up as a slightly longer path.
It felt extraneous, but I can leave it if you prefer?
config/initializers/carrier_wave.rb
Outdated
| @@ -14,5 +14,15 @@ | |||
| password: ENV['UPLOADER_PASSW'], | |||
| port: 22 | |||
| } | |||
| elsif Rails.env.staging? | |||
| config.storage = :aws | |||
| config.aws_bucket = ENV.fetch('S3_BUCKET_NAME', 'codebar-logos') | |||
There was a problem hiding this comment.
@jonodrew this is a small detail but can we rename this to sponsor-logos? codebar-logos make it sounds like it's our own logos. Also, will the config have to be updated for the products env above on line 6 and below?
There was a problem hiding this comment.
This is different in my most recent push because I realised I'd added more complexity where I didn't need it (I assumed we deployed simultaneously to all envs). I'm happy to rename the bucket!
jonodrew
force-pushed
the
aws-s3
branch
2 times, most recently
from
dac1297 to
e6db8e0
Compare
This is a longer-term plan, but evidences that it is low-complexity to move to S3 for image storage.
Before this is merged, we have to: