Add AWS S3 module with upload, download, and bucket scaffolding#160
Add AWS S3 module with upload, download, and bucket scaffolding#160maxn990 wants to merge 9 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
looks great so far, really thorough documentation for a somewhat more challenging aws service! could you update the tests for any changes you end up implementing in the service file?
| Add these variables to `.env` and `example.env`: | ||
|
|
||
| ``` | ||
| AWS_REGION=us-east-2 |
There was a problem hiding this comment.
should this always be us-east-2? some of our projects have some materials on us-east-1, not sure if we are trying to shift away from that and standardize it. if so, feel free to ignore this
There was a problem hiding this comment.
only ssf uses 1 i believe, i think we should try and standardize it to avoid some of the issues that ssf faced with that
|
|
||
| Because `mapBucket` uses a `Record<s3Buckets, string>`, TypeScript will produce a compile error if you add an enum entry without adding the corresponding mapping — catching missed steps at build time. | ||
|
|
||
| ## Required IAM Permissions |
There was a problem hiding this comment.
Do you think we should add instructions for how to create the bucket within AWS and where these permissions json should go in the AWS console (S3 -> bucket_name -> Permissions -> Bucket Policy -> Edit)
There was a problem hiding this comment.
we have that in the notion documentation already
chnnick
left a comment
chnnick
left a comment
There was a problem hiding this comment.
Honestly looks good to me, I like that we have READMEs for all the different modules I think they're super informative. There are already good input validation checks on the file name and size, my only comments are for possibly possibly two more checks on fields? The fixes are not necessary, but rather suggestions for more input validation. I understand this is a scaffolding repo so the MIME recommendation may limit the upload file-type flexibility.
dburkhart07
left a comment
dburkhart07
left a comment
There was a problem hiding this comment.
few more things, but looking much better!
3 participants
ℹ️ Issue
Closes #139
📝 Description
Write a short summary of what you added. Why is it important? Any member of C4C should be able to read this and understand your contribution -- not just your team members.
✔️ Verification
What steps did you take to verify your changes work? These should be clear enough for someone to be able to clone the branch and follow the steps themselves.
🏕️ (Optional) Future Work / Notes
Included two other small cleanup items - stopped tracking unnecessary nx workspace stuff and finally fixed ts config thing