default permission should assume aws v2 if both aws versions are in use #1986
Conversation
| @@ -120,7 +120,7 @@ def self.extended base | |||
| const_set('AWS_BASE_ERROR', | |||
| defined?(::Aws) ? Aws::Errors::ServiceError : AWS::Errors::Base) | |||
| const_set('DEFAULT_PERMISSION', | |||
| defined?(::AWS) ? :public_read : :'public-read') | |||
| defined?(::Aws) ? :'public-read' : :public_read) | |||
There was a problem hiding this comment.
Align the parameters of a method call if they span more than one line.
|
These conditionals would not exist if we had two different files, one for each branch. This PR doesn't change behavior or simplify the code, so I won't merge it for now. I am very interested in simplifying this into two files: Thank you very much for your contribution. |
|
This is a fix to a bug in code that has already been merged into master. @tute I assume from your later comment #1974 (comment) that you do plan on merging this after all? The question of separate classes was brought up already in the discussion on the PR that introduced AWS v2 support: #1903 (comment) |
|
Thanks! All done then. Where did you read about the upcoming deprecation of v1? I couldn't find docs related to that. |
|
@tute, I meant that eventually paperclip will want to deprecate AWS v1 support. I was assuming (correct me if I'm wrong) that once paperclip publishes a gem that supports both, we'll want to deprecate AWS v1 support a few versions later. (Reading that comment again now, I was also confused by what I meant.) |
We have a specific issue to discuss wether AWS v1 should be deprecated: #1997. |
|
Thank you for pointing me to that ticket. However, it has nothing to do with whether this PR should be merged. There is a bug in master. It should be fixed before we publish a gem. |
betesh
changed the title
|
@tute I still don't understand why this was closed. I have rebased it onto the latest master, where it is still an issue |
|
I just came from vacations, will look into it soon. |
|
Hi @betesh! I can't reopen the PR, because the branch seems to be gone from your repo. Want to pusblish it so we reopen? But before, why is this change necessary? Thank you! |
|
AWS v1: 'public_read' If you have both in you Gemfile, we use v2 but we use public_read, which is inconsistent and means that the default configuration is broken. |
No description provided.