search

spring-attic / spring-integration-kafka

Apache License 2.0
324 stars 180 forks source link

Use ConsumerProperties in KafkaMessageSource #283

Closed Walliee closed 5 years ago

Walliee commented 5 years ago

There are still a few more properties that need to be populated using setters on KafkaMessageSource e.g. messageConverter, payloadType, rawMessageHeader.

Does it make sense to create a configuration sub-class of ConsumerProperties?

Walliee commented 5 years ago

This PR doesn't honor commit callback, async commits, commit action logging configuration from ConsumerProperties. These can be partially or fully handled in #282

Changes related to topicPattern and topicPartitionsToAssign will be covered as part of #281

garyrussell commented 5 years ago

These can be partially or fully handled

Agreed.

Does it make sense to create a configuration sub-class of ConsumerProperties?

I think they can remain on the message source (we have a similar situation on the listener containers where a few properties are set there directly).

Let's see what @artembilan says.

Walliee commented 5 years ago

@artembilan I incorporated your feedback. Let me know if it looks good now.

garyrussell commented 5 years ago

LGTM, but we need a note in the what's new about the new default poll timeout - this can be a problem if they have several adapters and are using the default task scheduler which only has 10 threads since the thread will be tied up for 5 seconds if there are no new records.

artembilan commented 5 years ago

in the what's new about the new default poll timeout

Right, but that's already a Spring Kafka project mattern. Doesn't look like we document anything here..