-
Notifications
You must be signed in to change notification settings - Fork 153
feat(batch): add option to continue processing other group IDs on failure in SqsFifoPartialProcessor
#2590
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
feat(batch): add option to continue processing other group IDs on failure in SqsFifoPartialProcessor
#2590
Conversation
skipGroupOnError can be set only for SqsFifoPartialProcessor
…BatchProcessingOptions type
If skipGroupOnError is not true & have failed messages, we will short circuit the process. This is similar to old implementation
Because this will be more accurate with the test description
Ok, there are some merge conflicts. Will fix it and then make this ready. |
Hi @arnabrahman - thank you for your patience on this. I've just got back from PTO and will review this in the next 24hrs tops. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the work on this PR, the change is super clean and I really appreciate that you went the extra mile and also updated the docs!
I have left only a few minor comments, after which I am looking forward to merge the PR and include the feature in this week's release :)
Co-authored-by: Andrea Amorosi <[email protected]>
Co-authored-by: Andrea Amorosi <[email protected]>
Moving to ~L155 didn't work, had to put a new line. So, now it's at L156. For better readability, also added a new line after annotation.
…hman/aws-lambda-powertools-typescript into 2561-improve-sqs-fifo-processing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the thorough work on this PR, I really appreciate the attention to details and openness to work on iterations during the review.
Merging this now 🎉
PS: I've labelled two more issues (list) related to this utility as help wanted just in case you're interested. They're both in similar complexity and along the lines of this one.
Nice, i will take a look after finishing 1774 , @dreamorosi |
Summary
When using SQS FIFO, by default, we will stop processing at the first failure and mark unprocessed messages as failed to preserve ordering. However, this behavior may not be optimal for customers who wish to proceed with processing messages from a different group ID.
Here an option has been given to the customer to skip the default behavior.
Changes
skipGroupOnError
option that can be passed to theprocessPartialResponseSync
function.skipGroupOnError
is accepted only when the processor parameter is of typeSqsFifoPartialProcessor
. Example:skipGroupOnError
is not true, then short-circuit the process on first failure same as before.skipGroupOnError
is true, then continue processing on the failure. Only if the current group has a previous failure then skip processing the current record and logSqsFifoMessageGroupShortCircuitError
. Otherwise, process the record.skipGroupOnError
flagIssue number: #2561
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.