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
Set storage for BuildCommand and BuildEnvironment as private #7893
Conversation
@@ -50,6 +50,10 @@ def __init__(self, *args, **kwargs): | |||
'Ensure S3_BUILD_COMMANDS_STORAGE_BUCKET is defined.', | |||
) | |||
|
|||
self.bucket_acl = 'private' | |||
self.default_acl = 'private' | |||
self.querystring_auth = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make sure this works in dev, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could move the S3PrivateBucketMixin
I added in https://github.com/readthedocs/readthedocs-corporate/pull/1103/files#diff-ec70ada13c82176ac8d16446079e8f0966583f9d20ec9c1a80b3b95ae0d19fafR8-R17 to this repository and share this code with .com
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this works in dev. I think we can share a lot of code in .com now yes
We could update other PRs in .com to share these classes instead override them to also add the same parameters. I didn't copied these attrs because i thought everything would be públic in org, but makes sense to have these private. So |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. We can share more code with .com, tho
@@ -50,6 +50,10 @@ def __init__(self, *args, **kwargs): | |||
'Ensure S3_BUILD_COMMANDS_STORAGE_BUCKET is defined.', | |||
) | |||
|
|||
self.bucket_acl = 'private' | |||
self.default_acl = 'private' | |||
self.querystring_auth = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could move the S3PrivateBucketMixin
I added in https://github.com/readthedocs/readthedocs-corporate/pull/1103/files#diff-ec70ada13c82176ac8d16446079e8f0966583f9d20ec9c1a80b3b95ae0d19fafR8-R17 to this repository and share this code with .com
This is how we have them in .com.