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

JMS: WIP: Initial commit for fault strategies #2599

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dankristensen
Copy link
Contributor

WIP: Initial commit with 2 simple fault strategies for jms.

I need a little help, creating tests for these. I looked into how it is done for kafka, but it does not look like it can be done the same way here.

Please give me some feedback/hints for this.

@dankristensen
Copy link
Contributor Author

The build keeps failing here. It works fine when i build it locally. Does anybody have any clues?

@ozangunalp
Copy link
Collaborator

I've only skimmed the change, it looks good. But I need to look into something with how messages are dispatched for JMS connector. I am looking at the CI issues, I hope I found the fix. After that I'll look into this PR.

@ozangunalp ozangunalp force-pushed the feature/failure_strategies branch from dd48f6a to 549f06e Compare May 7, 2024 12:43
@dankristensen
Copy link
Contributor Author

That looks good @ozangunalp . I Will take a look at this tommorow. I still need to implement an additional failurestrategy, that is putting the message on a dlq. I just wanted the initial flow working first, and Got a little stück.

@dankristensen
Copy link
Contributor Author

Finally i have had time to look further into this. I have a problem to get the test LocalPropagationAckTest.testChannelWithNackOnMessageContextDlq to work correctly. It is still returning 2,3,3,5,6 and should "only" return 2,3,5,6.

I had to pass the JmsConnector into the failure strategy. I am not happy about this, but did not find any other way to do this. Hopefully you guys have some pointers for me, so i can avoid this.

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.

2 participants