Skip to content

Fixed rabbitmq plugin delay strategy by applying correct header #824

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

Closed
wants to merge 1 commit into from

Conversation

31vi5
Copy link

@31vi5 31vi5 commented Apr 17, 2019

The delay strategy creates a header array of application_headers which contained the x-delay instead of having the x-delay header directly. Screenshot of the message in the queue https://i.imgur.com/Qk4jyc4.png

The result was that the message was not correctly delayed. I've added the correct header to the message and then the delay started working. I've left the original property unchanged other than removing unnecessary type casting.

@makasim
Copy link
Member

makasim commented May 15, 2019

There are functional tests that cover the use of the plugin (1, 2, 3). They pass.

The issue should be investigated future. Maybe the tests are wrong, or your change is not relevant.

@31vi5
Copy link
Author

31vi5 commented May 15, 2019

Perhaps it might have been my misunderstanding of the feature. If I understand it correctly (which now differs from my understanding of the behavior at the time of posting the pull request), the message is sent to queue and then processed right away and then requeued with the delay, is that correct?

@makasim
Copy link
Member

makasim commented May 15, 2019

It depends on a lot of factors. Let me know what you are doing and what you use and what are the versions.

@stale
Copy link

stale bot commented Jun 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 24, 2019
@stale stale bot closed this Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants