Skip to content

Issue-3802: make kafkaTemplate field-var accessible in KafkaItemWriter's subcla… #3807

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

Conversation

adityausathe
Copy link
Contributor

@adityausathe adityausathe commented Nov 21, 2020

#3802
Added test to prevent future reversion of this change

KafkaItemWriter<String, String> kafkaItemWriter = new KafkaItemWriter<String, String>() {
@Override
protected void writeKeyValue(String key, String value) {
this.kafkaTemplate.flush();
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be changed to this.kafkaTemplate.sendDefault(key, value);, because currently the parameters are not used. Do you agree?

};
kafkaItemWriter.setKafkaTemplate(kafkaTemplate);
kafkaItemWriter.writeKeyValue("k", "v");
verify(kafkaTemplate).flush();
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to the previous comment, this should be updated to verify(this.kafkaTemplate).sendDefault("k", "v");.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense. I'll update the PR

@fmbenhassine fmbenhassine added in: infrastructure pr-for: enhancement status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter labels Nov 26, 2020
@fmbenhassine
Copy link
Contributor

Great! LGTM now. Rebased, squashed and merged as feb46fd. Thank you for your contribution 👍

@fmbenhassine fmbenhassine removed the status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter label Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants