Skip to content
This repository has been archived by the owner on Mar 30, 2023. It is now read-only.

GH-290: Add ProducerRecordCreator #291

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

garyrussell
Copy link
Contributor

Resolves #290

Copy link
Contributor

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

I wonder why do we need such a complex solution while we could just let to pass a ProducerRecord in the payload and check for that in the KafkaProducerMessageHandler before calling KafkaTemplate.

How such a record is built - not our responsibility already.

At least that is what we have in many other MH impls.

We can definitely merge this - just wanted to share some other point of view...

@garyrussell
Copy link
Contributor Author

See spring-cloud/spring-cloud-stream-binder-kafka#790

We would have to make changes in the binder and it still won't help with his customization of the output record. With this solution, the user is free to use your recently merged message handler customizer.

@artembilan
Copy link
Contributor

And why is that?

He creates a ProducerRecord in his code whatever way he needs it with all the possible record customization.
Returns that record from the microservice function and our KafkaProducerMessageHandler just propagates it as is into underlying KafkaTemplate.

We don't need any customizers. More over a solution with this creator still may have flaws not letting to get access to something what could be better done in the upstream transformer.
In other words: it is not a KafkaProducerMessageHandler responsibility to transform a payload "hard" way.

Again: I'm not against your solution which is simply lambda from end-user perspective.
I just want to share thoughts about what we have so far in other places.
Otherwise there is really no limits with Function-based fine-graining...

@garyrussell
Copy link
Contributor Author

Again; think about it in the context of the kafka binder, not an SI application.

What if all he wants to do is add a custom header - and use all the goodness of the binder to create all the other properties of the record?

Copy link
Contributor

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

OK. Fair enough!

Let's give it a shot! 😄

Although I would still implement a logic to accept a ProducerRecord...
We can do that in the separate issue though.

@artembilan artembilan merged commit fbb2765 into spring-attic:master Oct 30, 2019
@garyrussell garyrussell deleted the GH-290 branch October 30, 2019 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support custom Producer RecordMessageConverter
2 participants