Re: [DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[Luke Chen]

Hi Alexandre,

Thanks for the update.
It looks better and clear now.

I'll vote for it later.

Thank you for this improvement!
Luke

On Thu, Jun 9, 2022 at 3:33 PM Alexandre Garnier  wrote:

> Hi Luke,
>
> Thanks for the feedback.
>
> 1. Usually, in "Proposed Changes" section, we won't put PR link there.
>> Maybe you can take KIP-824 as a reference.
>>
> I don't know what to add more here than "Add the new option", it's a
> really simple straightforward modification.
>
> 2. Also, it'd be better you can provide some example reader-config and
>> formatter-config file.
>> And how they work within the script.
>>
> I did add examples.
>
> 3. If user provide both `--reader-config or --formatter-config` and
>> `--property` at the same time, how will we handle this case?
>> Could you add that into the KIP?
>>
> This was covered by the sentence "As for
> --producer-property/--consumer-property with
> --consumer.config/--producer.config, any value from option --property would
> override value from config file."
> I did add an example for this situation.
>
>
> Le jeu. 9 juin 2022 à 05:21, Luke Chen  a écrit :
>
>> Hi Alexandre,
>>
>> Thanks for the KIP.
>>
>> Some comments:
>>
>> 1. Usually, in "Proposed Changes" section, we won't put PR link there.
>> Maybe you can take KIP-824
>> 
>> as a reference.
>>
>> 2. Also, it'd be better you can provide some example reader-config and
>> formatter-config file.
>> And how they work within the script.
>>
>> 3. If user provide both `--reader-config or --formatter-config` and
>> `--property` at the same time, how will we handle this case?
>> Could you add that into the KIP?
>>
>> Thank you.
>> Luke
>>
>>
>> On Sun, May 29, 2022 at 2:32 AM Alexandre Garnier 
>> wrote:
>>
>>> Hi!
>>>
>>> Thanks for the feedback.
>>> It's a good point, I updated KIP accordingly and did put the
>>> dot-separated option in rejected alternatives.
>>>
>>> Le ven. 27 mai 2022 à 10:22, deng ziming  a
>>> écrit :
>>> >
>>> > Thanks for the KIP, this is a good improvement. I only have one minor
>>> suggestion.
>>> >
>>> > Currently many command line tools supports config file argument, but
>>> their name style is not unified, for example, most newly added tools are
>>> using --command-config, but ConsoleConsumer use —consumer.config。 I think
>>> we should unify the naming style from now on, I recommend us to use
>>> --reader-config and --formatter-config for the newly added arguments.
>>> >
>>> > --
>>> > Best,
>>> > Ziming
>>> >
>>> >
>>> > > On May 26, 2022, at 4:36 PM, Alexandre Garnier 
>>> wrote:
>>> > >
>>> > > Hello everyone,
>>> > >
>>> > > Any feedback on this KIP https://cwiki.apache.org/confluence/x/bBqhD
>>> ?
>>> > > It is a straightforward improvement without any impact on existing
>>> users,
>>> > > so not much to discuss besides maybe the option name.
>>> > >
>>> > > --
>>> > > Alex
>>> > >
>>> > >
>>> > > Le mer. 18 mai 2022 à 10:44, Alexandre Garnier  a
>>> écrit :
>>> > >
>>> > >> Hi everyone,
>>> > >>
>>> > >> I created a KIP to add a config file option of reader/formatter for
>>> > >> kafka-console-(consumer|producer).sh tools.
>>> > >> https://cwiki.apache.org/confluence/x/bBqhD
>>> > >>
>>> > >> Thanks for your feedback,
>>> > >> --
>>> > >> Alex
>>> > >>
>>> >
>>>
>>

Re: [DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[Alexandre Garnier]

Hi Luke,

Thanks for the feedback.

1. Usually, in "Proposed Changes" section, we won't put PR link there.
> Maybe you can take KIP-824 as a reference.
>
I don't know what to add more here than "Add the new option", it's a really
simple straightforward modification.

2. Also, it'd be better you can provide some example reader-config and
> formatter-config file.
> And how they work within the script.
>
I did add examples.

3. If user provide both `--reader-config or --formatter-config` and
> `--property` at the same time, how will we handle this case?
> Could you add that into the KIP?
>
This was covered by the sentence "As for
--producer-property/--consumer-property with
--consumer.config/--producer.config, any value from option --property would
override value from config file."
I did add an example for this situation.


Le jeu. 9 juin 2022 à 05:21, Luke Chen  a écrit :

> Hi Alexandre,
>
> Thanks for the KIP.
>
> Some comments:
>
> 1. Usually, in "Proposed Changes" section, we won't put PR link there.
> Maybe you can take KIP-824
> 
> as a reference.
>
> 2. Also, it'd be better you can provide some example reader-config and
> formatter-config file.
> And how they work within the script.
>
> 3. If user provide both `--reader-config or --formatter-config` and
> `--property` at the same time, how will we handle this case?
> Could you add that into the KIP?
>
> Thank you.
> Luke
>
>
> On Sun, May 29, 2022 at 2:32 AM Alexandre Garnier 
> wrote:
>
>> Hi!
>>
>> Thanks for the feedback.
>> It's a good point, I updated KIP accordingly and did put the
>> dot-separated option in rejected alternatives.
>>
>> Le ven. 27 mai 2022 à 10:22, deng ziming  a
>> écrit :
>> >
>> > Thanks for the KIP, this is a good improvement. I only have one minor
>> suggestion.
>> >
>> > Currently many command line tools supports config file argument, but
>> their name style is not unified, for example, most newly added tools are
>> using --command-config, but ConsoleConsumer use —consumer.config。 I think
>> we should unify the naming style from now on, I recommend us to use
>> --reader-config and --formatter-config for the newly added arguments.
>> >
>> > --
>> > Best,
>> > Ziming
>> >
>> >
>> > > On May 26, 2022, at 4:36 PM, Alexandre Garnier 
>> wrote:
>> > >
>> > > Hello everyone,
>> > >
>> > > Any feedback on this KIP https://cwiki.apache.org/confluence/x/bBqhD?
>> > > It is a straightforward improvement without any impact on existing
>> users,
>> > > so not much to discuss besides maybe the option name.
>> > >
>> > > --
>> > > Alex
>> > >
>> > >
>> > > Le mer. 18 mai 2022 à 10:44, Alexandre Garnier  a
>> écrit :
>> > >
>> > >> Hi everyone,
>> > >>
>> > >> I created a KIP to add a config file option of reader/formatter for
>> > >> kafka-console-(consumer|producer).sh tools.
>> > >> https://cwiki.apache.org/confluence/x/bBqhD
>> > >>
>> > >> Thanks for your feedback,
>> > >> --
>> > >> Alex
>> > >>
>> >
>>
>

Re: [DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[Luke Chen]

Hi Alexandre,

Thanks for the KIP.

Some comments:

1. Usually, in "Proposed Changes" section, we won't put PR link there.
Maybe you can take KIP-824

as a reference.

2. Also, it'd be better you can provide some example reader-config and
formatter-config file.
And how they work within the script.

3. If user provide both `--reader-config or --formatter-config` and
`--property` at the same time, how will we handle this case?
Could you add that into the KIP?

Thank you.
Luke


On Sun, May 29, 2022 at 2:32 AM Alexandre Garnier 
wrote:

> Hi!
>
> Thanks for the feedback.
> It's a good point, I updated KIP accordingly and did put the
> dot-separated option in rejected alternatives.
>
> Le ven. 27 mai 2022 à 10:22, deng ziming  a
> écrit :
> >
> > Thanks for the KIP, this is a good improvement. I only have one minor
> suggestion.
> >
> > Currently many command line tools supports config file argument, but
> their name style is not unified, for example, most newly added tools are
> using --command-config, but ConsoleConsumer use —consumer.config。 I think
> we should unify the naming style from now on, I recommend us to use
> --reader-config and --formatter-config for the newly added arguments.
> >
> > --
> > Best,
> > Ziming
> >
> >
> > > On May 26, 2022, at 4:36 PM, Alexandre Garnier 
> wrote:
> > >
> > > Hello everyone,
> > >
> > > Any feedback on this KIP https://cwiki.apache.org/confluence/x/bBqhD?
> > > It is a straightforward improvement without any impact on existing
> users,
> > > so not much to discuss besides maybe the option name.
> > >
> > > --
> > > Alex
> > >
> > >
> > > Le mer. 18 mai 2022 à 10:44, Alexandre Garnier  a
> écrit :
> > >
> > >> Hi everyone,
> > >>
> > >> I created a KIP to add a config file option of reader/formatter for
> > >> kafka-console-(consumer|producer).sh tools.
> > >> https://cwiki.apache.org/confluence/x/bBqhD
> > >>
> > >> Thanks for your feedback,
> > >> --
> > >> Alex
> > >>
> >
>

Re: [DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[Alexandre Garnier]

Hi!

Thanks for the feedback.
It's a good point, I updated KIP accordingly and did put the
dot-separated option in rejected alternatives.

Le ven. 27 mai 2022 à 10:22, deng ziming  a écrit :
>
> Thanks for the KIP, this is a good improvement. I only have one minor 
> suggestion.
>
> Currently many command line tools supports config file argument, but their 
> name style is not unified, for example, most newly added tools are using 
> --command-config, but ConsoleConsumer use —consumer.config。 I think we should 
> unify the naming style from now on, I recommend us to use --reader-config and 
> --formatter-config for the newly added arguments.
>
> --
> Best,
> Ziming
>
>
> > On May 26, 2022, at 4:36 PM, Alexandre Garnier  wrote:
> >
> > Hello everyone,
> >
> > Any feedback on this KIP https://cwiki.apache.org/confluence/x/bBqhD?
> > It is a straightforward improvement without any impact on existing users,
> > so not much to discuss besides maybe the option name.
> >
> > --
> > Alex
> >
> >
> > Le mer. 18 mai 2022 à 10:44, Alexandre Garnier  a écrit :
> >
> >> Hi everyone,
> >>
> >> I created a KIP to add a config file option of reader/formatter for
> >> kafka-console-(consumer|producer).sh tools.
> >> https://cwiki.apache.org/confluence/x/bBqhD
> >>
> >> Thanks for your feedback,
> >> --
> >> Alex
> >>
>

Re: [DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[deng ziming]

Thanks for the KIP, this is a good improvement. I only have one minor 
suggestion.

Currently many command line tools supports config file argument, but their name 
style is not unified, for example, most newly added tools are using 
--command-config, but ConsoleConsumer use —consumer.config。 I think we should 
unify the naming style from now on, I recommend us to use --reader-config and 
--formatter-config for the newly added arguments.

--
Best,
Ziming


> On May 26, 2022, at 4:36 PM, Alexandre Garnier  wrote:
> 
> Hello everyone,
> 
> Any feedback on this KIP https://cwiki.apache.org/confluence/x/bBqhD?
> It is a straightforward improvement without any impact on existing users,
> so not much to discuss besides maybe the option name.
> 
> -- 
> Alex
> 
> 
> Le mer. 18 mai 2022 à 10:44, Alexandre Garnier  a écrit :
> 
>> Hi everyone,
>> 
>> I created a KIP to add a config file option of reader/formatter for
>> kafka-console-(consumer|producer).sh tools.
>> https://cwiki.apache.org/confluence/x/bBqhD
>> 
>> Thanks for your feedback,
>> --
>> Alex
>> 

Re: [DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[Alexandre Garnier]

Hello everyone,

Any feedback on this KIP https://cwiki.apache.org/confluence/x/bBqhD?
It is a straightforward improvement without any impact on existing users,
so not much to discuss besides maybe the option name.

-- 
Alex


Le mer. 18 mai 2022 à 10:44, Alexandre Garnier  a écrit :

> Hi everyone,
>
> I created a KIP to add a config file option of reader/formatter for
> kafka-console-(consumer|producer).sh tools.
> https://cwiki.apache.org/confluence/x/bBqhD
>
> Thanks for your feedback,
> --
> Alex
>

[DISCUSS] KIP-840: Config file option for MessageReader/MessageFormatter in ConsoleProducer/ConsoleConsumer

[Alexandre Garnier]

Hi everyone,

I created a KIP to add a config file option of reader/formatter for
kafka-console-(consumer|producer).sh tools.
https://cwiki.apache.org/confluence/x/bBqhD

Thanks for your feedback,
-- 
Alex