The long awaited AWS v2 support PR #1903
Conversation
| def assert_forbidden_response(url) | ||
| Net::HTTP.get_response(URI.parse(url)) do |response| | ||
| assert_equal "403", response.code, | ||
| "Expected HTTP response code 403, got #{response.code}" |
There was a problem hiding this comment.
Align the parameters of a method call if they span more than one line.
|
I was about to release v4.3, but this should part of the release, right @jyurek? |
|
I'd love if we could use an adapter instead of adding conditionals everywhere we use AWS in the code. |
|
There's too many releases in 4.3, will release anyway, and we can add this in 4.4. |
|
|
||
| object.expects((defined?(::Aws) ? :upload_file : :write)) | ||
| .with(anything, | ||
| content_type: 'image/png', |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
… file, and FakeWeb needs to return XML
…aws-sdk v1 and aws-sdk v2
|
|
||
| object.expects((defined?(::Aws) ? :upload_file : :write)) | ||
| .with(anything, | ||
| content_type: 'image/png', |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
cc @jyurek to merge this PR |
|
I had some local failures related to regions, so I put in a fix for them, but this is merged. There are things I would clean up in the code, but I can't let that stop this PR at this point. We can fix them in the future. Thank you very much for working on this. |
|
Yay! 🎉 🎈 👏 Gonna go out in 4.4. |
|
@betesh There are failures on Travis. Can you take a look at them and see if there's anything that jumps out at you? https://travis-ci.org/thoughtbot/paperclip/jobs/76525495 @tute That sounds good, but I want to handle these errors first, of course. |
|
@jyurek What was the goal of b8221c6? If I understand correctly, it's to make the attachment URL use
OR
|
|
The goal was to get my local tests passing, but turns out I was wrong. I reverted and it's all good now. My bad. :) |
|
Just curious, do you mean tests in a rails app that uses paperclip, or the S3 Live specs? |
|
I mean the Paperclip specs themselves. Though the failure I saw was in the feature specs, so technically it was an app that uses paperclip. But that's nitpicking. |
|
@tute Looks like v4.4 is being skipped - will this officially be released in v5? |
Yes. If you are on Rails 4.2 you can grab the latest v5 beta. |
curl -L https://github.com/thoughtbot/paperclip/pull/1903.patch > 1903.patch git apply --3way 1903.patch Fixed conflict in lib/papeclip/storage/s3.rb by adding .freeze to string literals in http/https prefixing.
curl -L https://github.com/thoughtbot/paperclip/pull/1903.patch > 1903.patch git apply --3way 1903.patch Fixed conflict in lib/papeclip/storage/s3.rb by adding .freeze to string literals in http/https prefixing.
Thank you to @davetchen, who really did most of the work here.