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

Add NATS request/reply pattern processor #2354

Merged

Conversation

marshauf
Copy link
Contributor

Adds a Benthos processor component, which allows sending request to a NATS subject and awaits a response.

Based on the concept of https://docs.nats.io/nats-concepts/core-nats/reqreply

Developed alongside #2352 and I am also happy to hear any feedback.

Signed-off-by: Marcel Hauf <git@marcelhauf.name>
@insidegreen
Copy link

Hi @marshauf,

great stuff! I'd tested your PR ... works great!
Could you add a parameter to set the InboxPrefix on the NATS connection?

And one question: You're using a mutex to lock the NATS connection?! But as far as I can see, the NATS connection does the same internally ...
Is there another reason for your mutex?

@marshauf
Copy link
Contributor Author

Hi @insidegreen ,
thank you. No, I will most likely not do anything more, than maintenance for my PRs until they get accepted. If you add the functionality I can add it to this PR.

Yes, the reason is in the Close method. When Benthos shuts down or decides to reconnect, the NATS connection gets dropped and deleted/set to nil. The mutex avoids calling a method on a nil NATS Connection in the Process method.
Since I am not 100% sure Benthos will not call the Close method until all calls to the process Method return and it will not call the process Method until Close and Connect return, I added the mutex to be on the safe side.

@insidegreen
Copy link

Hey @marshauf ,

customer inbox prefix for the processor
chi-deutschland#1

Signed-off-by: Tom Hieß <insidegreen86@gmail.com>
Copy link
Collaborator

@Jeffail Jeffail left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @marshauf (and @insidegreen)

@Jeffail Jeffail merged commit e3cbc32 into redpanda-data:main Apr 15, 2024
1 of 3 checks passed
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.

None yet

3 participants