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

Ack partial of a batch of messages #255

Merged
merged 8 commits into from
Dec 16, 2022
Merged

Conversation

efiShtain
Copy link
Contributor

Description

When providing a handleMessageBatch function, code currently allows ack all messages or throw an error which will reject all messages.
The fix allows the user to return an array of successful messages which will be deleted from the queue, the rest of the messages will return to the queue

Motivation and Context

Added for personal use and response to issue 245
#245

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@oursodon
Copy link

I'd love to see this PR merged 🙏

@b0xtch
Copy link

b0xtch commented May 3, 2021

This would be really helpful

@mcflypl
Copy link

mcflypl commented May 6, 2021

I am waiting for merge this pull request :)

@ellisium
Copy link

Any feedback if it will be merged? And if yes, when will it be?

@franciscomemoli
Copy link

I need this too. Anyone using this lib needs it, is the only way to process in batch and handle erros.

@vyrtus15
Copy link

@nspragg @robinjmurphy UP

@markussiebert
Copy link

Would be nice to see this getting merged :-)

@chiarag
Copy link

chiarag commented Jun 14, 2022

We would be really chuffed if this would be merged ;) Any status on this?

@olizilla olizilla mentioned this pull request Jul 22, 2022
2 tasks
@nicholasgriffintn
Copy link
Member

I see there are a fair amount of comments on this, however, it is out of date by a little, would be good to update this and then we can look at it in a future milestone.

Copy link
Member

@nicholasgriffintn nicholasgriffintn left a comment

Choose a reason for hiding this comment

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

Tested this locally and it works as expected.

I have updated this to work with AWS SDK V3 and I've also added a test for if handleMessageBatch returns void, which should delete all of the messages.

@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants