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.

Sign up for GitHub

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

Jump to bottom

fix: extending message visibility for more than 10 entries does not fail #34

Merged
merged 1 commit into from
Apr 27, 2023
Merged

fix: extending message visibility for more than 10 entries does not fail #34

merged 1 commit into from
Apr 27, 2023

Conversation

philasmar
Copy link
Contributor

Issue #, if available:
DOTNET-6902

Description of changes:

  • Added InMemory logger to be able to assert on Log messages, since most exceptions are swallowed.
  • Updated the behavior to divide messages that need visibility renewal into batches of 10, which is the maximum supported by SQS.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ashovlin ashovlin mentioned this pull request Apr 21, 2023
Merged
normj
normj requested changes Apr 21, 2023
View reviewed changes
src/AWS.Messaging/SQS/SQSMessagePoller.cs Outdated Show resolved Hide resolved
src/AWS.Messaging/Services/IMessagePoller.cs Outdated Show resolved Hide resolved
@philasmar philasmar force-pushed the asmarp/extend-message-visibility branch 6 times, most recently from 34b6fa7 to e99c06a Compare April 24, 2023 15:36
@philasmar philasmar requested a review from normj April 24, 2023 15:37
normj
normj approved these changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@normj normj left a comment

Choose a reason for hiding this comment

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

I'm good after my small comment and you address Alex's comments.

src/AWS.Messaging/SQS/SQSMessagePoller.cs Show resolved Hide resolved
@philasmar philasmar force-pushed the asmarp/extend-message-visibility branch from e99c06a to bf61d36 Compare April 27, 2023 14:40
@philasmar philasmar requested a review from ashovlin April 27, 2023 14:41
@philasmar philasmar merged commit 216c1f5 into dev Apr 27, 2023
@philasmar philasmar deleted the asmarp/extend-message-visibility branch March 20, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@normj normj normj approved these changes

@ashovlin ashovlin ashovlin approved these changes

@96malhar 96malhar Awaiting requested review from 96malhar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@philasmar @ashovlin @normj