Skip to content
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

fix image uploads on s3/imgur #956

Merged
merged 1 commit into from Jun 4, 2013
Merged

fix image uploads on s3/imgur #956

merged 1 commit into from Jun 4, 2013

Conversation

ZogStriP
Copy link
Member

@ZogStriP ZogStriP commented Jun 4, 2013

Meta: Storing images in the cloud

Most of the work was done on S3 uploads. They will now work properly when using (or not) a region and will automagically create the bucket if it doesn't already exists.

Imgur mostly got a lifting and a few bug fixes (all props goes to @benbeltran).

  • Changed a few site settings
    • removed unused imgur_client_secret
    • updated imgur_endpoint to use https
    • removed default for s3_region as it is no longer needed (and was wrong)
  • Refactored upload.rb model and extracted all the methods into their own class
    • s3 for handling uploads to s3 buckets
    • local_store for handling uploads to a local directory
    • The idea is that these classes handles uploads and returns the url of the uploaded file.
  • Created a new SiteSettingMissing exception that is used to hopefully provide better error log message whenever a site setting is missing (only used in s3.rb and imgur.rb for now)
  • Updated the tests accordingly to the changes

@discoursebot
Copy link

You've signed the CLA, ZogStriP. Thank you! This pull request is ready for review.

SamSaffron added a commit that referenced this pull request Jun 4, 2013
@SamSaffron SamSaffron merged commit 56ee7cd into discourse:master Jun 4, 2013
@SamSaffron
Copy link
Member

:) awesome

@ZogStriP ZogStriP deleted the fix-image-upload-to-s3 branch June 4, 2013 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants