Hi Chia-Ping, Thanks for the KIP. I find the interface definition really polished and intuitive! One small question--I noticed the change of the package to "org.apache.kafka.clients.tool". It doesn't look like there's any precedent for using that package. We also use the "org.apache.kafka.common" package for the "MessageFormatter" interface, which is in some ways the equivalent pluggable interface for the console consumer.
Do we know if it's necessary to preserve the Checkstyle import limitations (which I'm assuming are what motivated the shift in package name)? It seems like it might be better to just relax that constraint in order to colocate the pluggable interfaces for our console producer/consumer. Cheers, Chris On Tue, Mar 7, 2023 at 6:30 AM Chia-Ping Tsai <chia7...@gmail.com> wrote: > hi Mickael > > > ?> configs) in the Compatibility, Deprecation, and Migration Plan, I > > guess these can be removed now. > > Done! thanks for feedback > > > Mickael Maison <mickael.mai...@gmail.com> 於 2023年3月7日 下午7:13 寫道： > > > > Hi Chia-Ping, > > > > The new API looks good. > > I still see mentions to configure(InputStream inputStream, Map<String, > > ?> configs) in the Compatibility, Deprecation, and Migration Plan, I > > guess these can be removed now. > > > > Thanks, > > Mickael > > > > On Fri, Mar 3, 2023 at 2:37 PM Chia-Ping Tsai <chia7...@apache.org> > wrote: > >> > >> Dear all, > >> > >> there are some changes for KIP-614 > >> > >> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158866569 > >> > >> 1) the interface RecordReader extends Configurable. > >> 2) the input stream is removed from RecordReader#configure method > >> 3) RecordReader#readRecords accept InputStream as argument, and the > returned type is changed from single ProducerRecord to > Iterator<ProducerRecord> > >> > >> Please take a look and then start to vote if you have free time. thanks. > >> > >> vote: https://lists.apache.org/thread/kjdtyfg5xytn60q0qvxhfopzmfp9tsxr > >
