[Issue 2512] S3 Presign URL #2563
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chouinar
reviewed
Oct 24, 2024
api/tests/lib/opportunity_attachment_test_files/test_file_2.txt
Outdated
Show resolved
Hide resolved
3 tasks
chouinar
reviewed
Oct 28, 2024
chouinar
reviewed
Oct 30, 2024
Comment on lines
65
to
68
| if s3_config.s3_endpoint_url: | ||
| pre_sign_file_loc = pre_sign_file_loc.replace( | ||
| s3_config.s3_endpoint_url, "http://localhost:4566" | ||
| ) |
There was a problem hiding this comment.
I'd suggest we add a comment calling out this hacky fix as only mattering locally due to docker path issues.
There was a problem hiding this comment.
Should i mark it as todo as well ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #{2512}
Time to review: 20 mins
Changes proposed
Added test files for opportunity attachment
Updated local seed db with func to upload test files to S3
Updated factories to set opportunity attachment "file_location" from any value in static list
Updated the response schema to return new field
download_pathwhich is the link to the pre-signed urlAdded default Expiration limit for the s3 url in s3 config
Added reusable function to get aws boto3 session for local development
Added new env variable for local development
Added fixture to upload test files
Added unit test. Tests if we can successfully download s3 file.
Context for reviewers
Return a pre-signed url for an attachment a user can then use to download the s3 file
Additional information