Skip to content

Upgrade S3 - Part 6 - #4836

Merged
lindseydew merged 7 commits into
mainfrom
ld/upgrade-aws-s3-part6
Aug 5, 2026
Merged

Upgrade S3 - Part 6 #4836
lindseydew merged 7 commits into
mainfrom
ld/upgrade-aws-s3-part6

Conversation

@lindseydew

@lindseydew lindseydew commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

What does this change?

This migrates more of the s3 methods in image loader

How should a reviewer test this change?

  • Test crops still work
  • Test upload a non-image

How can success be measured?

Who should look at this?

Tested? Documented?

  • locally by committer
  • locally by Guardian reviewer
  • on the Guardian's TEST environment
  • relevant documentation added or amended (if needed)

@lindseydew
lindseydew requested a review from a team as a code owner July 27, 2026 16:18
@lindseydew lindseydew added the maintenance Departmental tracking: maintenance work, not a fix or a feature label Jul 27, 2026
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

@@ -241,6 +241,7 @@ class ImageLoaderController(auth: Authentication,

def getPreSignedUploadUrlsAndTrack: Action[AnyContent] = AuthenticatedAndAuthorised.async { request =>
val expiration = DateTimeUtils.now().plusHours(1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is expiration still used at all, given it's no longer used in the preSignedUrl bit further down... if not please can it be removed... if it is still in use, please can signatureDuration be defined first and then expiration calculated by adding signatureDuration to DateTimeUtils.now()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's used to set the status in the a dynamo table:

expires = expiration.toEpochSecond, // TTL in case upload is never completed by client

@gu-prout

gu-prout Bot commented Aug 5, 2026

Copy link
Copy Markdown

Seen on auth, usage, image-loader, metadata-editor, thrall, leases, cropper, kahuna (merged by @lindseydew 10 minutes and 19 seconds ago) Please check your changes!

@gu-prout

gu-prout Bot commented Aug 5, 2026

Copy link
Copy Markdown

Seen on collections, media-api (merged by @lindseydew 12 minutes and 33 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants