Only delete SQS messages after at task successfully processes the message #277
Conversation
…sage Addresses RichardKnop#253 for SQS
Codecov Report
@@ Coverage Diff @@
## master #277 +/- ##
==========================================
- Coverage 43.64% 43.63% -0.02%
==========================================
Files 31 31
Lines 2715 2716 +1
==========================================
Hits 1185 1185
- Misses 1396 1397 +1
Partials 134 134
Continue to review full report at Codecov.
|
| // Delete message after consuming successfully | ||
| err := b.deleteOne(delivery) | ||
| err := taskProcessor.Process(sig) | ||
| if err != nil { |
There was a problem hiding this comment.
Isn't it a bug? It would only delete the task if task processor failed.
There was a problem hiding this comment.
Oh, it does appear that way! I believe this should be if err == nil!
There was a problem hiding this comment.
Not sure why the diff is misleading.
Code was changed here: 74f2b63
And code in the master branch looks fine: https://github.com/RichardKnop/machinery/blob/master/v1/brokers/sqs/sqs.go#L197
There was a problem hiding this comment.
I think this has been corrected in master branch already. Please check the code in the master branch if it is correct. I am not too familiar with the SQS broker code as I haven't written it (I know much more about AMQP and Redis which was originally written by me).
Addresses #253 for SQS