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
Support ACL
flag
#40
base: main
Are you sure you want to change the base?
Support ACL
flag
#40
Conversation
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.
Can you explain what this does, or give a link.
Best if we can keep the default behavior unchanged.
Also good to add some tests.
aioaws/s3.py
Outdated
@@ -181,6 +194,7 @@ def signed_upload_url( | |||
size: int, | |||
content_disp: bool = True, | |||
expires: Optional[datetime] = None, | |||
acl: ACLType = "public-read", |
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.
Best if we can keep the default unchanged.
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.
I took the example from: Example: Browser-Based Upload using HTTP POST and POST Policy
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.
And the ACL Definition
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
- Coverage 96.64% 96.03% -0.61%
==========================================
Files 10 10
Lines 626 631 +5
Branches 102 103 +1
==========================================
+ Hits 605 606 +1
- Misses 15 17 +2
- Partials 6 8 +2
Continue to review full report at Codecov.
|
Will add some thanks for sure. Do you mean set the |
Exactly |
Will do, thanks so much for the feedback |
This PR it's ready for review, I've been using my fork and the ACL flag it's working correctly. Thanks for this small library |
I also changed the name of
BaseConfigProtocol
toConfigProtocol