Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-15 Thread Chia-Ping Tsai
Hi lsmael I will update the KIP to replace “ org.apache.kafka.clients.tool” by “ org.apache.kafka.tools” due to following reasons. 1) it is more consistent with existent server/clients pluggable interface package 2) kafka tool jar is not expected to be imported to write code for pluggable

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-14 Thread Chia-Ping Tsai
Hi juma > Ismael Juma 於 2023年3月15日 上午3:04 寫道: > > Hi Chia, > > Regarding `org.apache.kafka.clients.tool`, a few comments: > > 1. Why is it in `clients`? We don't generally consider tools to be a client. > 2. Why is it `tool`? We have a package `org.apache.kafka.tools`, so it's a > bit odd

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-14 Thread Ismael Juma
Hi Chia, Regarding `org.apache.kafka.clients.tool`, a few comments: 1. Why is it in `clients`? We don't generally consider tools to be a client. 2. Why is it `tool`? We have a package `org.apache.kafka.tools`, so it's a bit odd that this one uses singular instead of plural. 3. Also, should we

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-14 Thread Chris Egerton
Fair enough  On Tue, Mar 14, 2023 at 12:23 PM Chia-Ping Tsai wrote: > > > > Chris Egerton 於 2023年3月15日 上午12:04 寫道: > > > > 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 >

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-14 Thread Chia-Ping Tsai
> Chris Egerton 於 2023年3月15日 上午12:04 寫道: > > 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

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-14 Thread Chris Egerton
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"

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-07 Thread Chia-Ping Tsai
hi Mickael > ?> configs) in the Compatibility, Deprecation, and Migration Plan, I > guess these can be removed now. Done! thanks for feedback > Mickael Maison 於 2023年3月7日 下午7:13 寫道: > > Hi Chia-Ping, > > The new API looks good. > I still see mentions to configure(InputStream inputStream, Map

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-07 Thread Mickael Maison
Hi Chia-Ping, The new API looks good. I still see mentions to configure(InputStream inputStream, Map 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 wrote: > > Dear all, > > there

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-03-03 Thread Chia-Ping Tsai
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

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-27 Thread Chia-Ping Tsai
Hi David > > - The new interface looks good to me. Note that the javadoc does not > reflect the new interface though. > - Could we precise how errors will be handled? For instance, say that the > iterator could translate the input stream to a record. Would calling the > next method on the

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-27 Thread David Jacot
Thanks for the update. - The new interface looks good to me. Note that the javadoc does not reflect the new interface though. - Could we precise how errors will be handled? For instance, say that the iterator could translate the input stream to a record. Would calling the next method on the

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-25 Thread Chia-Ping Tsai
On 2023/02/25 08:26:28 David Jacot wrote: > 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

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-25 Thread David Jacot
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.

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-22 Thread Chia-Ping Tsai
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

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-22 Thread Alexandre Dupriez
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? The question arises because the interface exposes a close() method which is never invoked by the

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-22 Thread Chia-Ping Tsai
> A minor comment. The SPI is currently used exclusively for the > ConsoleProducer. However, it exposes high-level methods which hint at > it being a generic component. What is the actual scope of the SPI > inside the Kafka codebase? Is it planned to be re-used in other tools? > Or is this

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-20 Thread Alexandre Dupriez
Hi Chia-Ping, Thank you for the KIP and apologies for missing it earlier. A minor comment. The SPI is currently used exclusively for the ConsoleProducer. However, it exposes high-level methods which hint at it being a generic component. What is the actual scope of the SPI inside the Kafka

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-18 Thread Chia-Ping Tsai
On 2023/02/18 08:44:05 Tom Bentley wrote: > Hi Chia-Ping, > > To be honest the stateful version, setting an input stream once using the > `readFrom(InputStream)` method and then repeatedly asking for the next > record using a parameterless `readRecord()`, seems a bit more natural to me > than

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-18 Thread Chia-Ping Tsai
Dear all, I noticed a issue after starting the vote :( The package "org.apache.kafka.common" disallows to import "org.apache.kafka.clients.producer.ProducerRecord". Hence, "org.apache.kafka.common.RecordReader" can't reference "org.apache.kafka.clients.producer.ProducerRecord" directly. It

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-18 Thread Tom Bentley
Hi Chia-Ping, To be honest the stateful version, setting an input stream once using the `readFrom(InputStream)` method and then repeatedly asking for the next record using a parameterless `readRecord()`, seems a bit more natural to me than `readRecord(InputStream inputStream)` being called

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-18 Thread Chia-Ping Tsai
On 2023/02/17 06:47:18 Luke Chen wrote: > Hi Chia-Ping, > > Thanks for the KIP! > > Overall LGTM, just one minor comment: > Could we log warning messages to users when using deprecated MessageReader? Sure. I will address it when implementing the KIP.

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Luke Chen
Hi Chia-Ping, Thanks for the KIP! Overall LGTM, just one minor comment: Could we log warning messages to users when using deprecated MessageReader? Thank you. Luke On Fri, Feb 17, 2023 at 2:04 PM Chia-Ping Tsai wrote: > hi Bentley > > I have updated the KIP to make RecordReader extend

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Chia-Ping Tsai
hi Bentley I have updated the KIP to make RecordReader extend Configurable. PTAL

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Chia-Ping Tsai
> 3. On the matter of configure(): While it doesn't add any functionality, it > would be more consistent with other plugins if this interface extended > Configurable, and the InputStream was then passed via some other method > (`readFrom(InputStream)` perhaps). If nothing else it would make it

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Chia-Ping Tsai
On 2023/02/16 11:26:04 Federico Valeri wrote: > Hi Chia-Ping, thanks for updating the KIP. > > I would only add that we plan to remove the old trait in the next > major release. I think it's better to be explicit about this. sure. I have added this description to doc about deprecation.

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Tom Bentley
Hi Chia-Ping, Thanks for the KIP. I have just a few minor comments: 1. The Javadoc for the new interface says "an `InputStream` received via `init`" but the interface doesn't have an init() method. I guess you meant configure()? 2. The Javadoc should mention the need for implementations to have

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Federico Valeri
Hi Chia-Ping, thanks for updating the KIP. I would only add that we plan to remove the old trait in the next major release. I think it's better to be explicit about this. On Thu, Feb 16, 2023 at 11:34 AM Chia-Ping Tsai wrote: > > Dear all, > > I have updated the KIP to address comments. PTAL >

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-16 Thread Chia-Ping Tsai
Dear all, I have updated the KIP to address comments. PTAL https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158866569

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-15 Thread Chia-Ping Tsai
On 2023/02/15 14:29:06 Ismael Juma wrote: > Hi. > > Is MessageReader the right name? The client apis tend to use the word > `Record` instead. In fact, `readMessage` returns a `ProducerRecord`. Make sense. “Record” is the better naming. I will update KIP to use “Record” instead of “message”

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-15 Thread Ismael Juma
Hi. Is MessageReader the right name? The client apis tend to use the word `Record` instead. In fact, `readMessage` returns a `ProducerRecord`. Also, I wonder if we it would be useful for the interface to support the usage of Serializers. That is, the interface could return ProducerRecord as long

Re: [DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2023-02-15 Thread Mickael Maison
Hi Chia-Ping, Sorry nobody replied to your thread earlier! I stumbled on this while looking at https://issues.apache.org/jira/browse/KAFKA-14525 (migrating CLI tools out of core). Your proposal still makes sense and would help migrating the ConsoleProducer tool. Can we specify when we expect to

[DISCUSS] KIP-641 An new java interface to replace 'kafka.common.MessageReader'

2020-07-07 Thread Chia-Ping Tsai
hi all, I would like to start the discussion for KIP-641. https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158866569 Many thanks, Chia-Ping