Skip to content

Upgrade S3 - Part 2 - #4832

Merged
lindseydew merged 1 commit into
mainfrom
ld/upgrade-aws-s3-part2
Aug 3, 2026
Merged

Upgrade S3 - Part 2#4832
lindseydew merged 1 commit into
mainfrom
ld/upgrade-aws-s3-part2

Conversation

@lindseydew

@lindseydew lindseydew commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this change?

This upgrades the store methods in the s3 class

How should a reviewer test this change?

  • Test can make a crop
  • Test images are stored

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 15:58
@lindseydew lindseydew changed the title Port to use store v2 Upgrade S3 - Part 2 Jul 27, 2026
@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

Deploy build 14783 of media-service::grid::all to TEST

All deployment options

From guardian/actions-riff-raff.

twrichards
twrichards approved these changes Jul 28, 2026
View reviewed changes
Comment thread common-lib/src/main/scala/com/gu/mediaservice/lib/aws/S3.scala
import software.amazon.awssdk.regions.Region
import software.amazon.awssdk.services.s3.S3Client
import software.amazon.awssdk.services.s3.model.{GetObjectResponse, NoSuchKeyException, GetObjectRequest => GetObjectRequestV2}
import software.amazon.awssdk.services.s3.model.{GetObjectResponse, HeadObjectRequest, HeadObjectResponse, NoSuchKeyException, GetObjectRequest => GetObjectRequestV2, PutObjectRequest => PutObjectRequestV2}

@twrichards twrichards Jul 28, 2026

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 there a reason we're aliasing the imports? I can understand if this is temporary to make things more explicit during the transition phase, but after that I think it probably leaves behind confusion as people might be looking at the code further down and be googling the class only to find there's no such things as PutObjectRequestV2 for example

@lindseydew lindseydew Jul 30, 2026

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.

Yeah, it was just to help me keep track of things whilst this is in flight. I have a branch to deprecate the v1 stuff after so it'll only be temporary

@twrichards twrichards mentioned this pull request Jul 28, 2026
Open
4 tasks
Base automatically changed from ld/upgrade-aws-s3-part1 to main August 3, 2026 09:00
@lindseydew
lindseydew merged commit 4c3139f into main Aug 3, 2026
5 of 7 checks passed
@lindseydew
lindseydew deleted the ld/upgrade-aws-s3-part2 branch August 3, 2026 09:21
@gu-prout

gu-prout Bot commented Aug 3, 2026

Copy link
Copy Markdown

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

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

Reviewers

@twrichards twrichards twrichards approved these changes

Assignees

No one assigned

Labels

maintenance Departmental tracking: maintenance work, not a fix or a feature Seen-on-auth Seen-on-collections Seen-on-cropper Seen-on-image-loader Seen-on-kahuna Seen-on-leases Seen-on-media-api Seen-on-metadata-editor Seen-on-thrall Seen-on-usage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lindseydew @twrichards