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

feat(batch): add support to SQS FIFO queues (SqsFifoPartialProcessor) #1934

Merged
merged 6 commits into from
Feb 20, 2023

Conversation

rubenfonseca
Copy link
Contributor

@rubenfonseca rubenfonseca commented Feb 16, 2023

Issue number: #1927

Summary

Changes

Please provide a summary of what's being changed

This PR adds support for SQS FIFO queues on the Batch Processing utility.

From docs:

If you're using this feature with a FIFO queue, your function should stop processing messages after the first failure and return all failed and unprocessed messages in batchItemFailures. This helps preserve the ordering of messages in your queue.

User experience

Please share what the user experience looks like before and after this change

Before this change, using the BatchProcessor with an SQS FIFO would result into undefined behaviour, as we were still continuing to process records after the first failure.

Using the new specialized class SqsFifoPartialProcessor makes the utility safe to use with SQS FIFO.

carbon (37)

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

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.

@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation tests labels Feb 16, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 16, 2023
@rubenfonseca rubenfonseca added the batch Batch processing utility label Feb 16, 2023
@github-actions github-actions bot added the feature New feature or functionality label Feb 16, 2023
@rubenfonseca rubenfonseca marked this pull request as ready for review February 16, 2023 13:37
@rubenfonseca rubenfonseca requested a review from a team as a code owner February 16, 2023 13:37
@rubenfonseca rubenfonseca requested review from heitorlessa and removed request for a team February 16, 2023 13:37
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

This is great!! Made some initial comments and spotted two issues. After lunch, I'll look over docs and tests.

Thank you @rubenfonseca <3

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2023

Codecov Report

Base: 97.45% // Head: 97.43% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (a1367e7) compared to base (75b6924).
Patch coverage: 91.89% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1934      +/-   ##
===========================================
- Coverage    97.45%   97.43%   -0.02%     
===========================================
  Files          144      146       +2     
  Lines         6629     6660      +31     
  Branches       475      478       +3     
===========================================
+ Hits          6460     6489      +29     
- Misses         132      134       +2     
  Partials        37       37              
Impacted Files Coverage Δ
aws_lambda_powertools/utilities/batch/types.py 72.72% <72.72%> (ø)
aws_lambda_powertools/utilities/batch/__init__.py 100.00% <100.00%> (ø)
aws_lambda_powertools/utilities/batch/base.py 95.12% <100.00%> (+0.44%) ⬆️
...ools/utilities/batch/sqs_fifo_partial_processor.py 100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/base.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

# GIVEN
first_record = SQSRecord(sqs_event_factory("success"))
second_record = SQSRecord(sqs_event_factory("fail"))
# this would normally suceed, but since it's a FIFO queue, it will be marked as failure
Copy link
Contributor

Choose a reason for hiding this comment

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

loved this comment. thank you for your great attention to detail @rubenfonseca

rubenfonseca and others added 2 commits February 20, 2023 11:31
Co-authored-by: Heitor Lessa <lessa@amazon.nl>
Signed-off-by: Ruben Fonseca <fonseka@gmail.com>
Signed-off-by: Heitor Lessa <lessa@amazon.nl>
@heitorlessa heitorlessa changed the title feat(batch): support SQS FIFO queues feat(batch): add support to SQS FIFO queues Feb 20, 2023
@heitorlessa heitorlessa changed the title feat(batch): add support to SQS FIFO queues feat(batch): add support to SQS FIFO queues (SqsFifoPartialProcessor) Feb 20, 2023
@rubenfonseca rubenfonseca merged commit a0f115c into aws-powertools:develop Feb 20, 2023
@rubenfonseca rubenfonseca deleted the rf/batch-sqs-fifo branch February 20, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch Batch processing utility documentation Improvements or additions to documentation feature New feature or functionality size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Handle SQS FIFO queues with the Bach Processing utility
3 participants