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

Change S3 storage default_acl setting to a default value private #5461

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

wvandeun
Copy link
Contributor

Since fastapi-storages v0.3.0 it requires that you set a valueAWS_DEFAULT_ACL.
In its documentation however, it is specified that it is optional setting and that the default value is private.

https://github.com/aminalaee/fastapi-storages/blob/edc624fb647e128e82669f19b195e670a4714dfd/fastapi_storages/s3.py#L39

This creates a problem if an Infrahub user does not explicitly provide a value for this setting.

In this PR we set the default value for this setting to private, which is also the default value that boto3 uses.

@wvandeun wvandeun added the group/backend Issue related to the backend (API Server, Git Agent) label Jan 15, 2025
@wvandeun wvandeun self-assigned this Jan 15, 2025
Copy link

codspeed-hq bot commented Jan 15, 2025

CodSpeed Performance Report

Merging #5461 will degrade performances by 25.65%

Comparing wvd-20240115-set-default-value-aws-default-acl (01578ca) with stable (42703f5)

Summary

❌ 1 (👁 1) regressions
✅ 9 untouched benchmarks

Benchmarks breakdown

Benchmark stable wvd-20240115-set-default-value-aws-default-acl Change
👁 test_schemabranch_duplicate 326.5 µs 439.2 µs -25.65%

@wvandeun wvandeun merged commit 3ca8bd6 into stable Jan 15, 2025
35 checks passed
@wvandeun wvandeun deleted the wvd-20240115-set-default-value-aws-default-acl branch January 15, 2025 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants