Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Added :use_accelerate_endpoint option when using S3 #2291

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS
@@ -1,5 +1,6 @@
master:

* Improvement: Added `:use_accelerate_endpoint` option when using S3 to enable [Amazon S3 Transfer Acceleration](http://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html)
* Improvement: make the fingerprint digest configurable per attachment. The
default remains MD5 but this will change in a future version because it is
not considered secure anymore against intentional file corruption. For more
Expand Down
15 changes: 15 additions & 0 deletions lib/paperclip/storage/s3.rb
Expand Up @@ -102,6 +102,8 @@ module Storage
# Redundancy Storage. RRS enables customers to reduce their
# costs by storing non-critical, reproducible data at lower
# levels of redundancy than Amazon S3's standard storage.
# * +use_accelerate_endpoint+: Use accelerate endpoint
# http://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html
#
# You can set storage class on a per style bases by doing the following:
# :s3_storage_class => {
Expand Down Expand Up @@ -152,6 +154,13 @@ def self.extended base
@options[:url] = @options[:url].inspect if @options[:url].is_a?(Symbol)

@http_proxy = @options[:http_proxy] || nil

if @options.has_key?(:use_accelerate_endpoint) &&
Gem::Version.new(Aws::VERSION) < Gem::Version.new("2.3.0")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use 4 (not 3) spaces for indenting a condition in an if statement spanning multiple lines.

raise LoadError, ":use_accelerate_endpoint is only available from aws-sdk version 2.3.0. Please upgrade aws-sdk to a newer version."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [144/80]

end

@use_accelerate_endpoint = @options[:use_accelerate_endpoint]
end

Paperclip.interpolates(:s3_alias_url) do |attachment, style|
Expand Down Expand Up @@ -232,6 +241,8 @@ def s3_interface
config[:proxy_uri] = URI::HTTP.build(proxy_opts)
end

config[:use_accelerate_endpoint] = true if use_accelerate_endpoint?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config[:use_accelerate_endpoint] = true if use_accelerate_endpoint?

is the same as

config[:use_accelerate_endpoint] = use_accelerate_endpoint?


[:access_key_id, :secret_access_key, :credential_provider, :credentials].each do |opt|
config[opt] = s3_credentials[opt] if s3_credentials[opt]
end
Expand All @@ -257,6 +268,10 @@ def s3_object style_name = default_style
s3_bucket.object style_name_as_path(style_name)
end

def use_accelerate_endpoint?
!!@use_accelerate_endpoint
end

def using_http_proxy?
!!@http_proxy
end
Expand Down
2 changes: 1 addition & 1 deletion paperclip.gemspec
Expand Up @@ -35,7 +35,7 @@ Gem::Specification.new do |s|
s.add_development_dependency('rspec', '~> 3.0')
s.add_development_dependency('appraisal')
s.add_development_dependency('mocha')
s.add_development_dependency('aws-sdk', '>= 2.0.34', '< 3.0')
s.add_development_dependency('aws-sdk', '>= 2.3.0', '< 3.0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this bump necessary? I wouldn't require it to everyone; the exception takes care of it for us. CI should anyway install latest version, as that's what we are specifying here.

s.add_development_dependency('bourne')
s.add_development_dependency('cucumber', '~> 1.3.18')
s.add_development_dependency('aruba', '~> 0.9.0')
Expand Down
53 changes: 53 additions & 0 deletions spec/paperclip/storage/s3_spec.rb
Expand Up @@ -282,6 +282,59 @@ class << @dummy
end
end

context "use_accelerate_endpoint", :if => use_accelerate_endpoint_option_is_available_in_aws_sdk? do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [102/80]
Use the new Ruby 1.9 hash syntax.

context "defaults to false" do
before do
rebuild_model(
storage: :s3,
s3_credentials: {},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what Hound wants here:

rebuild_model(
  storage: :s3,
  s3_credentials: {},
  bucket: "bucket",
  # ...
)

bucket: "bucket",
path: ":attachment/:basename:dotextension",
s3_host_name: "s3-ap-northeast-1.amazonaws.com",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

s3_region: "ap-northeast-1"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a comma after the last parameter of a multiline method call.

)
@dummy = Dummy.new
@dummy.avatar = stringy_file
@dummy.stubs(:new_record?).returns(false)
end

it "returns a url based on an :s3_host_name path" do
assert_match %r{^//s3-ap-northeast-1.amazonaws.com/bucket/avatars/data[^\.]},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [85/80]

@dummy.avatar.url

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the parameters of a method call if they span more than one line.

end

it "uses the S3 client with the use_accelerate_endpoint config is false" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [81/80]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [81/80]

expect(@dummy.avatar.s3_bucket.client.config.use_accelerate_endpoint).to be(false)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [90/80]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [90/80]

end
end

context "set to true", :if => use_accelerate_endpoint_option_is_available_in_aws_sdk? do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [92/80]
Use the new Ruby 1.9 hash syntax.

before do
rebuild_model(
storage: :s3,
s3_credentials: {},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

bucket: "bucket",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

path: ":attachment/:basename:dotextension",
s3_host_name: "s3-accelerate.amazonaws.com",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

s3_region: "ap-northeast-1",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

use_accelerate_endpoint: true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the elements of a hash literal if they span more than one line.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a comma after the last parameter of a multiline method call.

)
@dummy = Dummy.new
@dummy.avatar = stringy_file
@dummy.stubs(:new_record?).returns(false)
end

it "returns a url based on an :s3_host_name path" do
assert_match %r{^//s3-accelerate.amazonaws.com/bucket/avatars/data[^\.]},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [81/80]

@dummy.avatar.url

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the parameters of a method call if they span more than one line.

end

it "uses the S3 client with the use_accelerate_endpoint config is true" do
expect(@dummy.avatar.s3_bucket.client.config.use_accelerate_endpoint).to be(true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [89/80]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [89/80]

end
end
end

context "An attachment that uses S3 for storage and has styles that return different file types" do
before do
rebuild_model (aws2_add_region).merge storage: :s3,
Expand Down
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Expand Up @@ -38,6 +38,7 @@
config.include TestData
config.include Reporting
config.extend VersionHelper
config.extend ConditionalFilterHelper
config.mock_framework = :mocha
config.before(:all) do
rebuild_model
Expand Down
5 changes: 5 additions & 0 deletions spec/support/conditional_filter_helper.rb
@@ -0,0 +1,5 @@
module ConditionalFilterHelper
def use_accelerate_endpoint_option_is_available_in_aws_sdk?
(Gem::Version.new(Aws::VERSION) >= Gem::Version.new("2.3.0"))
end
end