-
Notifications
You must be signed in to change notification settings - Fork 27
Issue-147: Add feature to pause/resume SQSMessagePoller #179
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
Conversation
|
I exposed a configuration item, PollingControlToken, that can be used to start/stop message receipt, so that the BackgroundService (MessagePumpService) can remain unmodified. |
|
@jkwpappin Thanks for the PR! Could you please take a look at that? |
|
Thanks @philasmar I will take a look at the failing build |
|
@philasmar I've managed to reproduce this behavior with the failing test. But doing the restore, build and test separately as per the github actions steps, they fail: Going to keep investigating. Working theory is there might be a concurrency issue here that is manifesting itself for some reason when run with the -no-build flag 😬 |
|
@philasmar this seems to be a concurrency issue in the tests, where sometimes the background worker won't have completed it's work when the verification is called on the Moq. I can't reproduce the failure locally anymore... when running the tests they always seem to pass now, I did make some modifications to the tests for how they block. Are you able to approve the waiting build workflows to see if they are still failing with the latest changes to the tests? Can you also reproduce these test failures when you run them? Thanks. |
1b4ac7a to
3c6a6e2
Compare
|
I ran the tests again and got the same result: |
|
I ran them locally on my machine. The unit test passed but the integ test failed: |
| Assert.Equal("ErrorMessage", publishResponse.Result.InnerException.Message); | ||
| Assert.Equal("ErrorCode", ((EventBridgePutEventsException)publishResponse.Result.InnerException).ErrorCode); | ||
| Assert.Equal("Message failed to publish.", publishResponse.Message); | ||
| Assert.Equal("ErrorMessage", publishResponse.InnerException.Message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please fix the warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dereference of a possibly null reference.
|
Hey @philasmar shall I close this PR as you've now raised this one to replace it? |
|
Yes please. I was waiting for all the tests to pass on the new one before communicating it on this PR. |
|
This change has been released in https://www.nuget.org/packages/AWS.Messaging/0.20.0-preview |
Issue #147 :
Description of changes:
Use case Blue/Green deployments, where the inactive service can change and we want to stop/start the message consumption dependent upon the active service.
I have followed the contribution guideline and have aimed to follow the existing repositories coding style.
I added a PollingControlToken with Start()/Stop() methods that is setup initially by the MessageBusBuilder and stored in the MessageConfiguration, but is also registered as a singleton in the ServiceCollection.
Consumers can retrieve the PollingControlToken via DI and can then call the Start()/Stop() methods against it when they need to control the message receipt.
Testing:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.