You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Manikumar <ma...@gmail.com> on 2018/08/03 09:39:09 UTC
Re: [DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
Hi Dong,
In KIP-332 discussion, It was agreed to use "--command-config" option name
for passing config property file.
We can also use same name in here, to make it consistent across all tools.
Thanks,
On Thu, Jul 12, 2018 at 9:20 AM Dong Lin <li...@gmail.com> wrote:
> wiki page is currently read-only and it is unavailable for write operation.
> Will update it later.
>
> On Wed, Jul 11, 2018 at 8:48 PM, Dong Lin <li...@gmail.com> wrote:
>
> > Ah I see. Thanks for the suggestion. It is updated now.
> >
> > On Wed, Jul 11, 2018 at 8:13 PM, Ted Yu <yu...@gmail.com> wrote:
> >
> >> bq. the same approach used by "--config-file" in ConfigCommand.
> >>
> >> I should have copied more from the KIP.
> >> What I meant was that ConfigCommand doesn't use "--config-file" option.
> So
> >> 'same approach' implies StreamsResetter class, not ConfigCommand.
> >>
> >> I didn't mean to change ConfigCommand w.r.t. name of the option.
> >>
> >> Cheers
> >>
> >> On Wed, Jul 11, 2018 at 8:06 PM Dong Lin <li...@gmail.com> wrote:
> >>
> >> > Do you mean we should replace "--command-config" with "--config-file"
> in
> >> > ConfigCommand? There is backward compatibility concern with the
> change.
> >> I
> >> > am not sure the benefit of this change is worth the effort to
> deprecate
> >> the
> >> > old config. Maybe we should do it separately if more people thing it
> is
> >> > necessary?
> >> >
> >> > On Wed, Jul 11, 2018 at 8:01 PM, Ted Yu <yu...@gmail.com> wrote:
> >> >
> >> > > bq. "--config-file" in ConfigCommand.
> >> > >
> >> > > Please update the above - it should be StreamsResetter
> >> > >
> >> > >
> >> > > On Wed, Jul 11, 2018 at 7:59 PM Dong Lin <li...@gmail.com>
> wrote:
> >> > >
> >> > > > Hey Ted,
> >> > > >
> >> > > > Thanks much for the suggestion. Yeah "config-file" looks better
> than
> >> > > > "command-config". I have updated the KIP as suggested.
> >> > > >
> >> > > > Thanks,
> >> > > > Dong
> >> > > >
> >> > > > On Wed, Jul 11, 2018 at 5:57 PM, Ted Yu <yu...@gmail.com>
> >> wrote:
> >> > > >
> >> > > > > Looking at StreamsResetter.java :
> >> > > > >
> >> > > > > commandConfigOption = optionParser.accepts("config-file",
> >> > > > "Property
> >> > > > > file containing configs to be passed to admin cl
> >> > > > >
> >> > > > > Not sure you have considered naming the option in the above
> >> fashion.
> >> > > > >
> >> > > > > Probably add the above to Alternative section.
> >> > > > >
> >> > > > > Cheers
> >> > > > >
> >> > > > > On Wed, Jul 11, 2018 at 2:04 PM Dong Lin <li...@gmail.com>
> >> > wrote:
> >> > > > >
> >> > > > > > Hi all,
> >> > > > > >
> >> > > > > > I have created KIP-340: Allow kafka-reassign-partitions.sh and
> >> > > > > > kafka-log-dirs.sh to take admin client property file. See
> >> > > > > >
> >> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> > > > > 340%3A+Allow+kafka-reassign-partitions.sh+and+kafka-log-
> >> > > > > dirs.sh+to+take+admin+client+property+file
> >> > > > > > .
> >> > > > > >
> >> > > > > > This KIP provides a way to allow kafka-reassign-partitions.sh
> >> and
> >> > > > > > kafka-log-dirs.sh to talk to broker over SSL. Please review
> the
> >> KIP
> >> > > if
> >> > > > > you
> >> > > > > > have time.
> >> > > > > >
> >> > > > > >
> >> > > > > > Thanks!
> >> > > > > > Dong
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>
Re: [DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
Posted by Attila Sasvári <as...@apache.org>.
+1 (non-binding)
Thanks for the explanation.
In the meantime, I have also tested your patch with an SSL-enabled broker,
and kafka-log-dirs worked well.
Regards,
- Attila
On Sat, Aug 11, 2018 at 6:21 PM Dong Lin <li...@gmail.com> wrote:
> Hey Attila,
>
> Sorry for late reply.. I forgot to reply the email.
>
> I have manually tested it by connecting kafka-log-dirs.sh to SSL-enabled
> broker. Regarding integration test, currently
> SaslSslAdminClientIntegrationTest
> tests AdminClient's connection to SSL-enabled broker. In terms of
> connection to SSL-enabled broker, both kafka-reassign-partitions.sh and
> kafka-log-dirs.sh are just thin wrapper around AdminClient so it is
> probably not necessary to specifically test either scripts for
> SSL-connection capability. In the future if there is any issue, we will
> always add a test in addition to fixing the bug.
>
> Regards,
> Dong
>
> On Tue, Aug 7, 2018 at 5:28 AM, Attila Sasvári <as...@apache.org>
> wrote:
>
> > Hi Dong,
> >
> > Thanks for the KIP. +1 on using "command-config".
> >
> > Regarding the testing strategy:
> > - Is it planned to add new system test(s) to execute kafka-log-dirs.sh
> > (kafka-reassign-partitions.sh) against an SSL-enabled broker? I know that
> > the change is quite small, but it might be useful.
> >
> > Best,
> > Attila
> >
> > On Tue, Aug 7, 2018 at 7:16 AM Dong Lin <li...@gmail.com> wrote:
> >
> > > Hi all,
> > >
> > > Since we are going to use "command-config" as the option name in
> > KIP-332, I
> > > have updated the KIP to use "command-config" for consistency.
> > >
> > > Please comment if you think "config-file" is better.
> > >
> > > Thanks,
> > > Dong
> > >
> > > On Fri, Aug 3, 2018 at 2:39 AM, Manikumar <ma...@gmail.com>
> > > wrote:
> > >
> > > > Hi Dong,
> > > >
> > > > In KIP-332 discussion, It was agreed to use "--command-config" option
> > > name
> > > > for passing config property file.
> > > > We can also use same name in here, to make it consistent across all
> > > > tools.
> > > >
> > > > Thanks,
> > > >
> > > > On Thu, Jul 12, 2018 at 9:20 AM Dong Lin <li...@gmail.com>
> wrote:
> > > >
> > > > > wiki page is currently read-only and it is unavailable for write
> > > > operation.
> > > > > Will update it later.
> > > > >
> > > > > On Wed, Jul 11, 2018 at 8:48 PM, Dong Lin <li...@gmail.com>
> > wrote:
> > > > >
> > > > > > Ah I see. Thanks for the suggestion. It is updated now.
> > > > > >
> > > > > > On Wed, Jul 11, 2018 at 8:13 PM, Ted Yu <yu...@gmail.com>
> > wrote:
> > > > > >
> > > > > >> bq. the same approach used by "--config-file" in ConfigCommand.
> > > > > >>
> > > > > >> I should have copied more from the KIP.
> > > > > >> What I meant was that ConfigCommand doesn't use "--config-file"
> > > > option.
> > > > > So
> > > > > >> 'same approach' implies StreamsResetter class, not
> ConfigCommand.
> > > > > >>
> > > > > >> I didn't mean to change ConfigCommand w.r.t. name of the option.
> > > > > >>
> > > > > >> Cheers
> > > > > >>
> > > > > >> On Wed, Jul 11, 2018 at 8:06 PM Dong Lin <li...@gmail.com>
> > > wrote:
> > > > > >>
> > > > > >> > Do you mean we should replace "--command-config" with
> > > > "--config-file"
> > > > > in
> > > > > >> > ConfigCommand? There is backward compatibility concern with
> the
> > > > > change.
> > > > > >> I
> > > > > >> > am not sure the benefit of this change is worth the effort to
> > > > > deprecate
> > > > > >> the
> > > > > >> > old config. Maybe we should do it separately if more people
> > thing
> > > it
> > > > > is
> > > > > >> > necessary?
> > > > > >> >
> > > > > >> > On Wed, Jul 11, 2018 at 8:01 PM, Ted Yu <yu...@gmail.com>
> > > > wrote:
> > > > > >> >
> > > > > >> > > bq. "--config-file" in ConfigCommand.
> > > > > >> > >
> > > > > >> > > Please update the above - it should be StreamsResetter
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > On Wed, Jul 11, 2018 at 7:59 PM Dong Lin <
> lindong28@gmail.com
> > >
> > > > > wrote:
> > > > > >> > >
> > > > > >> > > > Hey Ted,
> > > > > >> > > >
> > > > > >> > > > Thanks much for the suggestion. Yeah "config-file" looks
> > > better
> > > > > than
> > > > > >> > > > "command-config". I have updated the KIP as suggested.
> > > > > >> > > >
> > > > > >> > > > Thanks,
> > > > > >> > > > Dong
> > > > > >> > > >
> > > > > >> > > > On Wed, Jul 11, 2018 at 5:57 PM, Ted Yu <
> > yuzhihong@gmail.com>
> > > > > >> wrote:
> > > > > >> > > >
> > > > > >> > > > > Looking at StreamsResetter.java :
> > > > > >> > > > >
> > > > > >> > > > > commandConfigOption =
> optionParser.accepts("config-
> > > > file",
> > > > > >> > > > "Property
> > > > > >> > > > > file containing configs to be passed to admin cl
> > > > > >> > > > >
> > > > > >> > > > > Not sure you have considered naming the option in the
> > above
> > > > > >> fashion.
> > > > > >> > > > >
> > > > > >> > > > > Probably add the above to Alternative section.
> > > > > >> > > > >
> > > > > >> > > > > Cheers
> > > > > >> > > > >
> > > > > >> > > > > On Wed, Jul 11, 2018 at 2:04 PM Dong Lin <
> > > lindong28@gmail.com
> > > > >
> > > > > >> > wrote:
> > > > > >> > > > >
> > > > > >> > > > > > Hi all,
> > > > > >> > > > > >
> > > > > >> > > > > > I have created KIP-340: Allow
> > kafka-reassign-partitions.sh
> > > > and
> > > > > >> > > > > > kafka-log-dirs.sh to take admin client property file.
> > See
> > > > > >> > > > > >
> > > > > >> > > > > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > >> > > > > 340%3A+Allow+kafka-reassign-partitions.sh+and+kafka-log-
> > > > > >> > > > > dirs.sh+to+take+admin+client+property+file
> > > > > >> > > > > > .
> > > > > >> > > > > >
> > > > > >> > > > > > This KIP provides a way to allow
> > > > kafka-reassign-partitions.sh
> > > > > >> and
> > > > > >> > > > > > kafka-log-dirs.sh to talk to broker over SSL. Please
> > > review
> > > > > the
> > > > > >> KIP
> > > > > >> > > if
> > > > > >> > > > > you
> > > > > >> > > > > > have time.
> > > > > >> > > > > >
> > > > > >> > > > > >
> > > > > >> > > > > > Thanks!
> > > > > >> > > > > > Dong
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
Posted by Dong Lin <li...@gmail.com>.
Hey Attila,
Sorry for late reply.. I forgot to reply the email.
I have manually tested it by connecting kafka-log-dirs.sh to SSL-enabled
broker. Regarding integration test, currently SaslSslAdminClientIntegrationTest
tests AdminClient's connection to SSL-enabled broker. In terms of
connection to SSL-enabled broker, both kafka-reassign-partitions.sh and
kafka-log-dirs.sh are just thin wrapper around AdminClient so it is
probably not necessary to specifically test either scripts for
SSL-connection capability. In the future if there is any issue, we will
always add a test in addition to fixing the bug.
Regards,
Dong
On Tue, Aug 7, 2018 at 5:28 AM, Attila Sasvári <as...@apache.org> wrote:
> Hi Dong,
>
> Thanks for the KIP. +1 on using "command-config".
>
> Regarding the testing strategy:
> - Is it planned to add new system test(s) to execute kafka-log-dirs.sh
> (kafka-reassign-partitions.sh) against an SSL-enabled broker? I know that
> the change is quite small, but it might be useful.
>
> Best,
> Attila
>
> On Tue, Aug 7, 2018 at 7:16 AM Dong Lin <li...@gmail.com> wrote:
>
> > Hi all,
> >
> > Since we are going to use "command-config" as the option name in
> KIP-332, I
> > have updated the KIP to use "command-config" for consistency.
> >
> > Please comment if you think "config-file" is better.
> >
> > Thanks,
> > Dong
> >
> > On Fri, Aug 3, 2018 at 2:39 AM, Manikumar <ma...@gmail.com>
> > wrote:
> >
> > > Hi Dong,
> > >
> > > In KIP-332 discussion, It was agreed to use "--command-config" option
> > name
> > > for passing config property file.
> > > We can also use same name in here, to make it consistent across all
> > > tools.
> > >
> > > Thanks,
> > >
> > > On Thu, Jul 12, 2018 at 9:20 AM Dong Lin <li...@gmail.com> wrote:
> > >
> > > > wiki page is currently read-only and it is unavailable for write
> > > operation.
> > > > Will update it later.
> > > >
> > > > On Wed, Jul 11, 2018 at 8:48 PM, Dong Lin <li...@gmail.com>
> wrote:
> > > >
> > > > > Ah I see. Thanks for the suggestion. It is updated now.
> > > > >
> > > > > On Wed, Jul 11, 2018 at 8:13 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > > > >
> > > > >> bq. the same approach used by "--config-file" in ConfigCommand.
> > > > >>
> > > > >> I should have copied more from the KIP.
> > > > >> What I meant was that ConfigCommand doesn't use "--config-file"
> > > option.
> > > > So
> > > > >> 'same approach' implies StreamsResetter class, not ConfigCommand.
> > > > >>
> > > > >> I didn't mean to change ConfigCommand w.r.t. name of the option.
> > > > >>
> > > > >> Cheers
> > > > >>
> > > > >> On Wed, Jul 11, 2018 at 8:06 PM Dong Lin <li...@gmail.com>
> > wrote:
> > > > >>
> > > > >> > Do you mean we should replace "--command-config" with
> > > "--config-file"
> > > > in
> > > > >> > ConfigCommand? There is backward compatibility concern with the
> > > > change.
> > > > >> I
> > > > >> > am not sure the benefit of this change is worth the effort to
> > > > deprecate
> > > > >> the
> > > > >> > old config. Maybe we should do it separately if more people
> thing
> > it
> > > > is
> > > > >> > necessary?
> > > > >> >
> > > > >> > On Wed, Jul 11, 2018 at 8:01 PM, Ted Yu <yu...@gmail.com>
> > > wrote:
> > > > >> >
> > > > >> > > bq. "--config-file" in ConfigCommand.
> > > > >> > >
> > > > >> > > Please update the above - it should be StreamsResetter
> > > > >> > >
> > > > >> > >
> > > > >> > > On Wed, Jul 11, 2018 at 7:59 PM Dong Lin <lindong28@gmail.com
> >
> > > > wrote:
> > > > >> > >
> > > > >> > > > Hey Ted,
> > > > >> > > >
> > > > >> > > > Thanks much for the suggestion. Yeah "config-file" looks
> > better
> > > > than
> > > > >> > > > "command-config". I have updated the KIP as suggested.
> > > > >> > > >
> > > > >> > > > Thanks,
> > > > >> > > > Dong
> > > > >> > > >
> > > > >> > > > On Wed, Jul 11, 2018 at 5:57 PM, Ted Yu <
> yuzhihong@gmail.com>
> > > > >> wrote:
> > > > >> > > >
> > > > >> > > > > Looking at StreamsResetter.java :
> > > > >> > > > >
> > > > >> > > > > commandConfigOption = optionParser.accepts("config-
> > > file",
> > > > >> > > > "Property
> > > > >> > > > > file containing configs to be passed to admin cl
> > > > >> > > > >
> > > > >> > > > > Not sure you have considered naming the option in the
> above
> > > > >> fashion.
> > > > >> > > > >
> > > > >> > > > > Probably add the above to Alternative section.
> > > > >> > > > >
> > > > >> > > > > Cheers
> > > > >> > > > >
> > > > >> > > > > On Wed, Jul 11, 2018 at 2:04 PM Dong Lin <
> > lindong28@gmail.com
> > > >
> > > > >> > wrote:
> > > > >> > > > >
> > > > >> > > > > > Hi all,
> > > > >> > > > > >
> > > > >> > > > > > I have created KIP-340: Allow
> kafka-reassign-partitions.sh
> > > and
> > > > >> > > > > > kafka-log-dirs.sh to take admin client property file.
> See
> > > > >> > > > > >
> > > > >> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > >> > > > > 340%3A+Allow+kafka-reassign-partitions.sh+and+kafka-log-
> > > > >> > > > > dirs.sh+to+take+admin+client+property+file
> > > > >> > > > > > .
> > > > >> > > > > >
> > > > >> > > > > > This KIP provides a way to allow
> > > kafka-reassign-partitions.sh
> > > > >> and
> > > > >> > > > > > kafka-log-dirs.sh to talk to broker over SSL. Please
> > review
> > > > the
> > > > >> KIP
> > > > >> > > if
> > > > >> > > > > you
> > > > >> > > > > > have time.
> > > > >> > > > > >
> > > > >> > > > > >
> > > > >> > > > > > Thanks!
> > > > >> > > > > > Dong
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
Posted by Attila Sasvári <as...@apache.org>.
Hi Dong,
Thanks for the KIP. +1 on using "command-config".
Regarding the testing strategy:
- Is it planned to add new system test(s) to execute kafka-log-dirs.sh
(kafka-reassign-partitions.sh) against an SSL-enabled broker? I know that
the change is quite small, but it might be useful.
Best,
Attila
On Tue, Aug 7, 2018 at 7:16 AM Dong Lin <li...@gmail.com> wrote:
> Hi all,
>
> Since we are going to use "command-config" as the option name in KIP-332, I
> have updated the KIP to use "command-config" for consistency.
>
> Please comment if you think "config-file" is better.
>
> Thanks,
> Dong
>
> On Fri, Aug 3, 2018 at 2:39 AM, Manikumar <ma...@gmail.com>
> wrote:
>
> > Hi Dong,
> >
> > In KIP-332 discussion, It was agreed to use "--command-config" option
> name
> > for passing config property file.
> > We can also use same name in here, to make it consistent across all
> > tools.
> >
> > Thanks,
> >
> > On Thu, Jul 12, 2018 at 9:20 AM Dong Lin <li...@gmail.com> wrote:
> >
> > > wiki page is currently read-only and it is unavailable for write
> > operation.
> > > Will update it later.
> > >
> > > On Wed, Jul 11, 2018 at 8:48 PM, Dong Lin <li...@gmail.com> wrote:
> > >
> > > > Ah I see. Thanks for the suggestion. It is updated now.
> > > >
> > > > On Wed, Jul 11, 2018 at 8:13 PM, Ted Yu <yu...@gmail.com> wrote:
> > > >
> > > >> bq. the same approach used by "--config-file" in ConfigCommand.
> > > >>
> > > >> I should have copied more from the KIP.
> > > >> What I meant was that ConfigCommand doesn't use "--config-file"
> > option.
> > > So
> > > >> 'same approach' implies StreamsResetter class, not ConfigCommand.
> > > >>
> > > >> I didn't mean to change ConfigCommand w.r.t. name of the option.
> > > >>
> > > >> Cheers
> > > >>
> > > >> On Wed, Jul 11, 2018 at 8:06 PM Dong Lin <li...@gmail.com>
> wrote:
> > > >>
> > > >> > Do you mean we should replace "--command-config" with
> > "--config-file"
> > > in
> > > >> > ConfigCommand? There is backward compatibility concern with the
> > > change.
> > > >> I
> > > >> > am not sure the benefit of this change is worth the effort to
> > > deprecate
> > > >> the
> > > >> > old config. Maybe we should do it separately if more people thing
> it
> > > is
> > > >> > necessary?
> > > >> >
> > > >> > On Wed, Jul 11, 2018 at 8:01 PM, Ted Yu <yu...@gmail.com>
> > wrote:
> > > >> >
> > > >> > > bq. "--config-file" in ConfigCommand.
> > > >> > >
> > > >> > > Please update the above - it should be StreamsResetter
> > > >> > >
> > > >> > >
> > > >> > > On Wed, Jul 11, 2018 at 7:59 PM Dong Lin <li...@gmail.com>
> > > wrote:
> > > >> > >
> > > >> > > > Hey Ted,
> > > >> > > >
> > > >> > > > Thanks much for the suggestion. Yeah "config-file" looks
> better
> > > than
> > > >> > > > "command-config". I have updated the KIP as suggested.
> > > >> > > >
> > > >> > > > Thanks,
> > > >> > > > Dong
> > > >> > > >
> > > >> > > > On Wed, Jul 11, 2018 at 5:57 PM, Ted Yu <yu...@gmail.com>
> > > >> wrote:
> > > >> > > >
> > > >> > > > > Looking at StreamsResetter.java :
> > > >> > > > >
> > > >> > > > > commandConfigOption = optionParser.accepts("config-
> > file",
> > > >> > > > "Property
> > > >> > > > > file containing configs to be passed to admin cl
> > > >> > > > >
> > > >> > > > > Not sure you have considered naming the option in the above
> > > >> fashion.
> > > >> > > > >
> > > >> > > > > Probably add the above to Alternative section.
> > > >> > > > >
> > > >> > > > > Cheers
> > > >> > > > >
> > > >> > > > > On Wed, Jul 11, 2018 at 2:04 PM Dong Lin <
> lindong28@gmail.com
> > >
> > > >> > wrote:
> > > >> > > > >
> > > >> > > > > > Hi all,
> > > >> > > > > >
> > > >> > > > > > I have created KIP-340: Allow kafka-reassign-partitions.sh
> > and
> > > >> > > > > > kafka-log-dirs.sh to take admin client property file. See
> > > >> > > > > >
> > > >> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >> > > > > 340%3A+Allow+kafka-reassign-partitions.sh+and+kafka-log-
> > > >> > > > > dirs.sh+to+take+admin+client+property+file
> > > >> > > > > > .
> > > >> > > > > >
> > > >> > > > > > This KIP provides a way to allow
> > kafka-reassign-partitions.sh
> > > >> and
> > > >> > > > > > kafka-log-dirs.sh to talk to broker over SSL. Please
> review
> > > the
> > > >> KIP
> > > >> > > if
> > > >> > > > > you
> > > >> > > > > > have time.
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > Thanks!
> > > >> > > > > > Dong
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
Posted by Dong Lin <li...@gmail.com>.
Hi all,
Since we are going to use "command-config" as the option name in KIP-332, I
have updated the KIP to use "command-config" for consistency.
Please comment if you think "config-file" is better.
Thanks,
Dong
On Fri, Aug 3, 2018 at 2:39 AM, Manikumar <ma...@gmail.com> wrote:
> Hi Dong,
>
> In KIP-332 discussion, It was agreed to use "--command-config" option name
> for passing config property file.
> We can also use same name in here, to make it consistent across all
> tools.
>
> Thanks,
>
> On Thu, Jul 12, 2018 at 9:20 AM Dong Lin <li...@gmail.com> wrote:
>
> > wiki page is currently read-only and it is unavailable for write
> operation.
> > Will update it later.
> >
> > On Wed, Jul 11, 2018 at 8:48 PM, Dong Lin <li...@gmail.com> wrote:
> >
> > > Ah I see. Thanks for the suggestion. It is updated now.
> > >
> > > On Wed, Jul 11, 2018 at 8:13 PM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > >> bq. the same approach used by "--config-file" in ConfigCommand.
> > >>
> > >> I should have copied more from the KIP.
> > >> What I meant was that ConfigCommand doesn't use "--config-file"
> option.
> > So
> > >> 'same approach' implies StreamsResetter class, not ConfigCommand.
> > >>
> > >> I didn't mean to change ConfigCommand w.r.t. name of the option.
> > >>
> > >> Cheers
> > >>
> > >> On Wed, Jul 11, 2018 at 8:06 PM Dong Lin <li...@gmail.com> wrote:
> > >>
> > >> > Do you mean we should replace "--command-config" with
> "--config-file"
> > in
> > >> > ConfigCommand? There is backward compatibility concern with the
> > change.
> > >> I
> > >> > am not sure the benefit of this change is worth the effort to
> > deprecate
> > >> the
> > >> > old config. Maybe we should do it separately if more people thing it
> > is
> > >> > necessary?
> > >> >
> > >> > On Wed, Jul 11, 2018 at 8:01 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > >> >
> > >> > > bq. "--config-file" in ConfigCommand.
> > >> > >
> > >> > > Please update the above - it should be StreamsResetter
> > >> > >
> > >> > >
> > >> > > On Wed, Jul 11, 2018 at 7:59 PM Dong Lin <li...@gmail.com>
> > wrote:
> > >> > >
> > >> > > > Hey Ted,
> > >> > > >
> > >> > > > Thanks much for the suggestion. Yeah "config-file" looks better
> > than
> > >> > > > "command-config". I have updated the KIP as suggested.
> > >> > > >
> > >> > > > Thanks,
> > >> > > > Dong
> > >> > > >
> > >> > > > On Wed, Jul 11, 2018 at 5:57 PM, Ted Yu <yu...@gmail.com>
> > >> wrote:
> > >> > > >
> > >> > > > > Looking at StreamsResetter.java :
> > >> > > > >
> > >> > > > > commandConfigOption = optionParser.accepts("config-
> file",
> > >> > > > "Property
> > >> > > > > file containing configs to be passed to admin cl
> > >> > > > >
> > >> > > > > Not sure you have considered naming the option in the above
> > >> fashion.
> > >> > > > >
> > >> > > > > Probably add the above to Alternative section.
> > >> > > > >
> > >> > > > > Cheers
> > >> > > > >
> > >> > > > > On Wed, Jul 11, 2018 at 2:04 PM Dong Lin <lindong28@gmail.com
> >
> > >> > wrote:
> > >> > > > >
> > >> > > > > > Hi all,
> > >> > > > > >
> > >> > > > > > I have created KIP-340: Allow kafka-reassign-partitions.sh
> and
> > >> > > > > > kafka-log-dirs.sh to take admin client property file. See
> > >> > > > > >
> > >> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >> > > > > 340%3A+Allow+kafka-reassign-partitions.sh+and+kafka-log-
> > >> > > > > dirs.sh+to+take+admin+client+property+file
> > >> > > > > > .
> > >> > > > > >
> > >> > > > > > This KIP provides a way to allow
> kafka-reassign-partitions.sh
> > >> and
> > >> > > > > > kafka-log-dirs.sh to talk to broker over SSL. Please review
> > the
> > >> KIP
> > >> > > if
> > >> > > > > you
> > >> > > > > > have time.
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > Thanks!
> > >> > > > > > Dong
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>