You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Dong Lin <li...@gmail.com> on 2018/07/11 21:03:47 UTC
[DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
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
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>
Re: [DISCUSS] KIP-340: Allow kafka-reassign-partitions.sh and
kafka-log-dirs.sh to take admin client property file
Posted by Manikumar <ma...@gmail.com>.
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 Dong Lin <li...@gmail.com>.
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 Dong Lin <li...@gmail.com>.
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 Ted Yu <yu...@gmail.com>.
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 Dong Lin <li...@gmail.com>.
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 Ted Yu <yu...@gmail.com>.
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 Dong Lin <li...@gmail.com>.
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 Ted Yu <yu...@gmail.com>.
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
>