Hi Chia-Ping, Thanks for the KIP.
I find the configure method in the proposed interface a bit weird for a few reasons. First, it has a default implementation which suggests that it is optional but it is not because the InputStream is required. Second, it diverges from the Configurable interface. Third, it is strange to pass an InputStream to a configure method. Did we consider using two methods instead of one? We could have configure coming from Configurable et setInputStream to set the InputStream. Another option would be to have a method which takes the input stream and returns an iterator to consume the records. Cheers, David Le mer. 22 févr. 2023 à 11:53, Chia-Ping Tsai <chia7...@apache.org> a écrit : > > > On 2023/02/22 10:01:29 Alexandre Dupriez wrote: > > Hi Chia-Ping, > > > > Thanks for your answer. Apologies I should have been clearer in the > > previous message. What I meant is, is there a plan to use the SPI more > > broadly inside the Kafka codebase? > > no, there is no plan to reuse the SPI. > > > The question arises because the interface exposes a close() method > > which is never invoked by the ConsoleProducer. Hence, although we need > > to keep this method to maintain compatibility of the SPI with its > > current implementations > > yep, you are right. the close() method is never executed by the > ConsoleProducer. The ConsolerConsumer has similar issue ( > https://github.com/apache/kafka/pull/8978). I will fix it. > > > we should perhaps clarify that this method is > > not used/deprecated, unless it is intended to be used in the future. > > I prefer to keep the close() since the new interface is similar to > Deserializer. The close() method can be used to notify/release something > when the console is going to be down. > >
