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

S3Boto3Storage: bucket_params not used #257

Closed
mattayes opened this Issue Jan 27, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@mattayes
Copy link
Contributor

mattayes commented Jan 27, 2017

On line 363 of the _get_or_create_bucket() method of S3Boto3Storage, bucket_params is defined and includes the bucket's ACL setting. However, when bucket.create() is called, bucket_params isn't used and ACL is set explicitly.

Additionally, if a region is specified, it's added to bucket_params but not set when bucket.create() is called. This makes all created buckets default to us-standard.

Here's the offending lines:

bucket_params = {'ACL': self.bucket_acl}
region_name = self.connection.meta.client.meta.region_name
if region_name != 'us-east-1':
    bucket_params['CreateBucketConfiguration'] = {'LocationConstraint': region_name}
bucket.create(ACL=self.bucket_acl)

I believe the bucket.create() call should be bucket.create(**bucket_params). I'll submit a PR for this in the next five minutes or so.

mattayes pushed a commit to mattayes/django-storages that referenced this issue Jan 27, 2017

@jschneier

This comment has been minimized.

Copy link
Owner

jschneier commented Feb 28, 2017

Yep.

jschneier added a commit that referenced this issue Feb 28, 2017

#257: S3Boto3Storage bucket_params (#258)
* #257 Use bucket_params when creating bucket

* Add self to AUTHORS

jschneier added a commit that referenced this issue Feb 28, 2017

@jschneier

This comment has been minimized.

Copy link
Owner

jschneier commented Feb 28, 2017

Added a test. Thanks.

@pyup-bot pyup-bot referenced this issue Jul 18, 2017

Closed

Initial Update #42

@pyup-bot pyup-bot referenced this issue Oct 3, 2017

Merged

Initial Update #90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.