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

chore: Add text to S3 object metadata #21

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Conversation

cmaddox5
Copy link
Contributor

@cmaddox5 cmaddox5 commented Aug 27, 2024

Added text sent to Polly to the S3 object metadata:
Screenshot 2024-08-27 at 10 07 49 AM


@cmaddox5 cmaddox5 requested a review from a team as a code owner August 27, 2024 14:08
@cmaddox5 cmaddox5 changed the title chore: Add text to object metadata chore: Add text to S3 object metadata Aug 27, 2024
ExAws.S3.put_object_copy(bucket, key, bucket, key, metadata_directive: :REPLACE)
ExAws.S3.put_object_copy(bucket, key, bucket, key,
metadata_directive: :REPLACE,
meta: [{:text, text}]
Copy link
Contributor

@digitalcora digitalcora Aug 27, 2024

Choose a reason for hiding this comment

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

What was the reason we're replacing the metadata with this copy? Is that needed to update the timestamp? It doesn't appear to be part of what S3 terms "metadata" — my admittedly brief searching suggests it's almost impossible to not update the timestamp when writing an object, regardless of the method.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that without directive, the call would error out because it was a no-op. Telling it to replace the metadata makes it an actual change, so it completes and updates the timestamp, which is what we actually care about.

Copy link
Contributor

@sloanelybutsurely sloanelybutsurely left a comment

Choose a reason for hiding this comment

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

lgtm!

@cmaddox5 cmaddox5 merged commit 3363946 into main Aug 27, 2024
2 checks passed
@cmaddox5 cmaddox5 deleted the cm/add-text-metadata branch August 27, 2024 14:55
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.

4 participants