fix: batch processing util#155
Conversation
more SQS specific focus Update for sqs_batch_processor interface
Codecov Report
@@ Coverage Diff @@
## develop #155 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 28 28
Lines 806 827 +21
Branches 76 77 +1
=========================================
+ Hits 806 827 +21
Continue to review full report at Codecov.
|
| records = event["Records"] | ||
|
|
||
| with processor(records, record_handler): | ||
| processor.process() |
There was a problem hiding this comment.
Can you add a logger.debug here to provide a message that you're processing another record. We don't need to log the record for security reasons, just a message suffices.
There was a problem hiding this comment.
This is outside of the loop for processing single records, but I've added debug logging elsewhere which should suffice.
|
|
||
| **Background** | ||
|
|
||
| When using SQS as a Lambda event source mapping, functions can be triggered with a batch of messages from SQS. If the Lambda function fails when processing the batch, |
There was a problem hiding this comment.
Let's add some line breaks to make it easier to read. The docs theme kinda forces you to do that or else it cramps the lines
| There are 2 ways to use this utility for processing SQS messages: | ||
|
|
||
| ### PartialSQSProcessor | ||
| **With a decorator:** |
There was a problem hiding this comment.
Use a sub-heading #### as opposed to bold text - It allows customers and us to easily reference it in conversations
| **With a decorator:** | ||
|
|
||
| SQS integration with Lambda is one of the most well established ones and pretty useful when building asynchronous applications. One common approach to maximize the performance of this integration is to enable the batch processing feature, resulting in higher throughput with less invocations. | ||
| Using the `sqs_batch_processor` decorator with your lambda handler function, you provide a `record_handler` which is responsible for processing individual messages. It should raise an exception if |
There was a problem hiding this comment.
Let's add line breaks here to make it easier to read in our docs theme
| ``` | ||
|
|
||
| A *naive* approach to solving this problem is to delete successful records from the queue before redriving's phase. The `PartialSQSProcessor` class offers this solution both as context manager and middleware, removing all successful messages from the queue case one or more failures occurred during lambda's execution. Two examples are provided below, displaying the behavior of this class. | ||
| **With a context manager:** |
There was a problem hiding this comment.
Use a sub-heading #### as opposed to bold text - It allows customers and us to easily reference it in conversations
heitorlessa
left a comment
There was a problem hiding this comment.
Suggestions to break middlewares.py functionalities into base.py and sqs.py to be closer to their respective implementations, debug logging, and docs to make it easier to read.
I'll work on the docs as it's a quick one for me
heitorlessa
left a comment
There was a problem hiding this comment.
Looks great! Docs look super clean now too, and great to catch that failed record bug before it's released :)
Issue #, if available:
Description of changes:
Simplified the documentation, primarily by focusing more on the SQS specific implementation. Added a sqs_batch_processor interface to simplify the common use case. Fixed an issue where no exception was being raised for failed messages, which would have caused failed messages to be deleted from the queue.
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.