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

Do not disable delayed delivery since currently the FIFO queue is anyway always required by the transport #347

Closed
wants to merge 1 commit into from

Conversation

danielmarbach
Copy link
Contributor

@danielmarbach danielmarbach commented Feb 23, 2024

The comment in the code was misleading since the delayed message pump is always initialized which checks for the existence of the fifo queue.

@danielmarbach danielmarbach requested a review from kentdr February 23, 2024 15:04
@danielmarbach danielmarbach marked this pull request as ready for review February 23, 2024 15:04
@andreasohlund
Copy link
Member

FYI The failing test has been fixed in #292 so either wait for that to be merged or merge this one into that branch

The docs seems to be a bit of in regards to serializer and need for the error queue to be set so I will fix that

https://docs.particular.net/nservicebus/hosting/aws-lambda-simple-queue-service/

@danielmarbach
Copy link
Contributor Author

@andreasohlund would you be able to cherry-pick this change over there?

@andreasohlund
Copy link
Member

I've cherry-picked the commit, I'll take care of updating the docs.

https://docs.particular.net/nservicebus/hosting/aws-lambda-simple-queue-service/#configuration-delayed-retries
only thing that needs to change right?

@danielmarbach
Copy link
Contributor Author

That's all I'm aware of yes

@bording
Copy link
Member

bording commented Mar 5, 2024

Merged as part of #292

@bording bording closed this Mar 5, 2024
@bording bording deleted the small-fixes branch March 5, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants