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

Add flexibility to s3 #99

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add flexibility to s3 #99

wants to merge 4 commits into from

Conversation

jacobc2700
Copy link
Contributor

@jacobc2700 jacobc2700 commented Jul 15, 2024

Change/PR 1: add flexibility to s3
make changes to config.ts with a new native enum (non-zod) for bucket names.
enum names should be all caps, bucket names should not
in the enum, add current resume bucket rp-2024-resumes, but also add one for speakers (rp-2024-speakers)
pass that enum as a parameter into the get/postUrl methods in s3-utils.ts
use that enum's value to get the actual bucket name, instead of the config/env value (you can delete the current hardcoded bucket from config)

@jacobc2700 jacobc2700 changed the title s3 flex Add flexibility to s3 Jul 15, 2024
Copy link
Contributor

@AydanPirani AydanPirani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacobc2700 have you tested this?

import { getSignedUrl } from "@aws-sdk/s3-request-presigner";
import { createPresignedPost } from "@aws-sdk/s3-presigned-post";

export async function postResumeUrl(userId: string, client: S3) {
export async function postResumeUrl(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rename the get and post function names because now these functions will be getting URLs for both resumes and for speaker images

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it's no longer just getResumeURL/postResumeURL

@AydanPirani
Copy link
Contributor

hello! @jacobc2700 bumping this back up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants