Skip to content

Conversation

@ellenmuller
Copy link
Contributor

@ellenmuller ellenmuller commented Jan 26, 2026

What does this change?

This PR actions some comments from my previous PR, #4593, most notably this one #4593 (comment).

We now loop through the records sent from SQS, and if there's an error they get captured as BatchItemFailures and returned at the end of the handler. This is to avoid an error happening halfway through the loop, and the whole batch ending on the DLQ. This is recommended by AWS here https://docs.aws.amazon.com/prescriptive-guidance/latest/lambda-event-filtering-partial-batch-responses-for-sqs/best-practices-partial-batch-responses.html.

I've also added a script to send a batch of records and we can see that the handler correctly loops through them. (Do note that the write to the S3 Vector Store will fail because we're trying to write duplicate keys! This script is just useful for testing the loop behaviour and perhaps may be useful in the future if we start skipping images already embedded)

I also added a case class EmbedderMessage for the messageBody we send to SQS from image-loader for extra type safety.

@ellenmuller ellenmuller added the feature Departmental tracking: work on a new feature label Jan 26, 2026
@github-actions
Copy link

github-actions bot commented Jan 26, 2026

@ellenmuller ellenmuller marked this pull request as ready for review January 26, 2026 17:18
@ellenmuller ellenmuller requested a review from a team as a code owner January 26, 2026 17:18
@ellenmuller ellenmuller changed the title change batch size to 1 and raise error if more than 1 Loop through records from SQS Jan 27, 2026
case Tiff => Left("Image file type is not supported. File type: Tiff")
}
}
object EmbedderMessage {
Copy link
Member

Choose a reason for hiding this comment

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

👌

@ellenmuller ellenmuller force-pushed the em-lambda-fix branch 2 times, most recently from 1e90bf8 to 89cb191 Compare January 28, 2026 10:10
@@ -0,0 +1,43 @@
#!/bin/bash
set -euo pipefail
Copy link
Member

Choose a reason for hiding this comment

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

think it's worth a note (comment?) about what this does! sends a batch containing the same message 10 times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah fair enough - I added something in the PR description so I'll add that as a comment

Copy link
Member

@joelochlann joelochlann left a comment

Choose a reason for hiding this comment

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

Nice!

@gu-prout
Copy link

gu-prout bot commented Jan 29, 2026

Seen on metadata-editor (merged by @ellenmuller 8 minutes and 54 seconds ago) Please check your changes!

@gu-prout
Copy link

gu-prout bot commented Jan 29, 2026

Seen on auth, usage, image-loader, cropper, collections, kahuna (merged by @ellenmuller 8 minutes and 59 seconds ago) Please check your changes!

@gu-prout
Copy link

gu-prout bot commented Jan 29, 2026

Seen on thrall, leases, media-api (merged by @ellenmuller 9 minutes and 5 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.

3 participants