You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Aneel Nazareth <an...@confluent.io> on 2020/04/01 17:30:21 UTC
Re: [DISCUSS] KIP-574: CLI Dynamic Configuration with file input
I have a PR that I think is ready for review here:
https://github.com/apache/kafka/pull/8184
Feedback would be most welcome!
On Mon, Mar 30, 2020 at 6:57 PM Colin McCabe <cm...@apache.org> wrote:
>
> Just as a note about whether we should support "-" as a synonym for STDIN... I agree it's kind of inconsistent.
>
> It may not be that big of a deal to drop support for STDIN. A lot of UNIX shells make it easy to create a temporary file in scripts-- for example, you could use a "here document" ( https://en.wikipedia.org/wiki/Here_document )
>
> best,
> Colin
>
>
> On Fri, Mar 27, 2020, at 07:46, Aneel Nazareth wrote:
> > Update: I have simplified the KIP down to just adding the single new
> > --add-config-file option. Thanks for your input, everyone!
> >
> > On Thu, Mar 26, 2020 at 10:13 AM Aneel Nazareth <an...@confluent.io> wrote:
> > >
> > > Hi Kamal,
> > >
> > > Thanks for taking a look at this KIP.
> > >
> > > Unfortunately the user actually can't pass the arguments on the
> > > command line using the existing --add-config option if the values are
> > > complex structures that contain commas. --add-config assumes that
> > > commas separate distinct configuration properties. There's a
> > > workaround using square brackets ("[a,b,c]") for simple lists, but it
> > > doesn't work for things like nested lists or JSON values.
> > >
> > > The motivation for allowing STDIN as well as files is to enable
> > > grep/pipe workflows in scripts without creating a temporary file. I
> > > don't know if such workflows will end up being common, and hopefully
> > > someone with a complex enough use case to require it would also be
> > > familiar with techniques for securely creating and cleaning up
> > > temporary files.
> > >
> > > I'm okay with excluding the option to allow STDIN in the name of
> > > consistency, if the consensus thinks that's wise. Anyone else have
> > > opinions on this?
> > >
> > > On Thu, Mar 26, 2020 at 9:02 AM Kamal Chandraprakash
> > > <ka...@gmail.com> wrote:
> > > >
> > > > Hi Colin,
> > > >
> > > > We should not support STDIN to maintain uniformity across scripts. If the
> > > > user wants to pass the arguments in command line,
> > > > they can always use the existing --add-config option.
> > > >
> > > >
> > > >
> > > >
> > > > On Thu, Mar 26, 2020 at 7:20 PM David Jacot <dj...@confluent.io> wrote:
> > > >
> > > > > Rajini has made a good point. I don't feel strong for either ways but if
> > > > > people
> > > > > are confused by this, it is probably better without it.
> > > > >
> > > > > Best,
> > > > > David
> > > > >
> > > > > On Thu, Mar 26, 2020 at 7:23 AM Colin McCabe <cm...@apache.org> wrote:
> > > > >
> > > > > > Hi Kamal,
> > > > > >
> > > > > > Are you suggesting that we not support STDIN here? I have mixed
> > > > > feelings.
> > > > > >
> > > > > > I think the ideal solution would be to support "-" in these tools
> > > > > whenever
> > > > > > a file argument was expected. But that would be a bigger change than
> > > > > what
> > > > > > we're talking about here. Maybe you are right and we should keep it
> > > > > simple
> > > > > > for now.
> > > > > >
> > > > > > best,
> > > > > > Colin
> > > > > >
> > > > > > On Wed, Mar 25, 2020, at 01:24, Kamal Chandraprakash wrote:
> > > > > > > STDIN wasn't standard practice in other scripts like
> > > > > > > kafka-console-consumer.sh, kafka-console-producer.sh and kafka-acls.sh
> > > > > > > in which the props file is accepted via consumer.config /
> > > > > > producer.config /
> > > > > > > command-config parameter.
> > > > > > >
> > > > > > > Shouldn't we have to maintain the uniformity across scripts?
> > > > > > >
> > > > > > > On Mon, Mar 16, 2020 at 4:13 PM David Jacot <dj...@confluent.io>
> > > > > wrote:
> > > > > > >
> > > > > > > > Hi Aneel,
> > > > > > > >
> > > > > > > > Thanks for the updated KIP. I have made a second pass over it and the
> > > > > > > > KIP looks good to me.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > David
> > > > > > > >
> > > > > > > > On Tue, Mar 10, 2020 at 9:39 PM Aneel Nazareth <an...@confluent.io>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > After reading a bit more about it in the Kubernetes case, I think
> > > > > > it's
> > > > > > > > > reasonable to do this and be explicit that we're ignoring the
> > > > > value,
> > > > > > > > > just deleting all keys that appear in the file.
> > > > > > > > >
> > > > > > > > > I've updated the KIP wiki page to reflect that:
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-574%3A+CLI+Dynamic+Configuration+with+file+input
> > > > > > > > >
> > > > > > > > > And updated my sample PR:
> > > > > > > > > https://github.com/apache/kafka/pull/8184
> > > > > > > > >
> > > > > > > > > If there are no further comments, I'll request a vote in a few
> > > > > days.
> > > > > > > > >
> > > > > > > > > Thanks for the feedback!
> > > > > > > > >
> > > > > > > > > On Mon, Mar 9, 2020 at 1:24 PM Aneel Nazareth <an...@confluent.io>
> > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi David,
> > > > > > > > > >
> > > > > > > > > > Is the expected behavior that the keys are deleted without
> > > > > > checking the
> > > > > > > > > values?
> > > > > > > > > >
> > > > > > > > > > Let's say I had this file new.properties:
> > > > > > > > > > a=1
> > > > > > > > > > b=2
> > > > > > > > > >
> > > > > > > > > > And ran:
> > > > > > > > > >
> > > > > > > > > > bin/kafka-configs --bootstrap-server localhost:9092 \
> > > > > > > > > > --entity-type brokers --entity-default \
> > > > > > > > > > --alter --add-config-file new.properties
> > > > > > > > > >
> > > > > > > > > > It seems clear what should happen if I run this immediately:
> > > > > > > > > >
> > > > > > > > > > bin/kafka-configs --bootstrap-server localhost:9092 \
> > > > > > > > > > --entity-type brokers --entity-default \
> > > > > > > > > > --alter --delete-config-file new.properties
> > > > > > > > > >
> > > > > > > > > > (Namely that both a and b would now have no values in the config)
> > > > > > > > > >
> > > > > > > > > > But what if this were run in-between:
> > > > > > > > > >
> > > > > > > > > > bin/kafka-configs --bootstrap-server localhost:9092 \
> > > > > > > > > > --entity-type brokers --entity-default \
> > > > > > > > > > --alter --add-config a=3
> > > > > > > > > >
> > > > > > > > > > Would it be surprising if the key/value pair a=3 was deleted,
> > > > > even
> > > > > > > > > > though the config that is in the file is a=1? Or would that be
> > > > > > > > > > expected?
> > > > > > > > > >
> > > > > > > > > > On Mon, Mar 9, 2020 at 1:02 PM David Jacot <dj...@confluent.io>
> > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Colin,
> > > > > > > > > > >
> > > > > > > > > > > Yes, you're right. This is weird but convenient because you
> > > > > don't
> > > > > > > > have
> > > > > > > > > to
> > > > > > > > > > > duplicate
> > > > > > > > > > > the "keys". I was thinking about the kubernetes API which
> > > > > allows
> > > > > > to
> > > > > > > > > create
> > > > > > > > > > > a Pod
> > > > > > > > > > > based on a file and allows to delete it as well with the same
> > > > > > file. I
> > > > > > > > > have
> > > > > > > > > > > always found
> > > > > > > > > > > this convenient, especially when doing local tests.
> > > > > > > > > > >
> > > > > > > > > > > Best,
> > > > > > > > > > > David
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Mar 9, 2020 at 6:35 PM Colin McCabe <
> > > > > cmccabe@apache.org>
> > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Aneel,
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for the KIP. I like the idea.
> > > > > > > > > > > >
> > > > > > > > > > > > You mention that "input from STDIN can be used instead of a
> > > > > > file on
> > > > > > > > > > > > disk." The example given in the KIP seems to suggest that
> > > > > the
> > > > > > > > > command
> > > > > > > > > > > > defaults to reading from STDIN if no argument is given to
> > > > > > > > > --add-config-file.
> > > > > > > > > > > >
> > > > > > > > > > > > I would argue against this particular command-line pattern.
> > > > > > From
> > > > > > > > the
> > > > > > > > > > > > user's point of view, if they mess up and forget to supply an
> > > > > > > > > argument, or
> > > > > > > > > > > > for some reason the parser doesn't treat something like an
> > > > > > > > argument,
> > > > > > > > > the
> > > > > > > > > > > > program will appear to hang in a confusing way.
> > > > > > > > > > > >
> > > > > > > > > > > > Instead, it would be better to follow the traditional UNIX
> > > > > > pattern
> > > > > > > > > where a
> > > > > > > > > > > > dash indicates that STDIN should be read. So
> > > > > > "--add-config-file -"
> > > > > > > > > would
> > > > > > > > > > > > indicate that the program should read form STDIN. This would
> > > > > > be
> > > > > > > > > difficult
> > > > > > > > > > > > to trigger accidentally, and more in line with the
> > > > > traditional
> > > > > > > > > conventions.
> > > > > > > > > > > >
> > > > > > > > > > > > On Mon, Mar 9, 2020, at 08:47, David Jacot wrote:
> > > > > > > > > > > > > I wonder if we should also add a `--delete-config-file` as
> > > > > a
> > > > > > > > > counterpart
> > > > > > > > > > > > of
> > > > > > > > > > > > > `--add-config-file`. It would be a bit weird to use a
> > > > > > properties
> > > > > > > > > file in
> > > > > > > > > > > > > this case as the values are not necessary but it may be
> > > > > > handy to
> > > > > > > > > have the
> > > > > > > > > > > > > possibility to remove the configurations which have been
> > > > > set.
> > > > > > > > Have
> > > > > > > > > you
> > > > > > > > > > > > > considered this?
> > > > > > > > > > > >
> > > > > > > > > > > > Hi David,
> > > > > > > > > > > >
> > > > > > > > > > > > That's an interesting idea. However, I think it might be
> > > > > > confusing
> > > > > > > > > to
> > > > > > > > > > > > users to supply a file, and then have the values supplied in
> > > > > > that
> > > > > > > > > file be
> > > > > > > > > > > > ignored. Is there really a case where we need to do this (as
> > > > > > > > > opposed to
> > > > > > > > > > > > creating a file with blank values, or just passing the keys
> > > > > to
> > > > > > > > > > > > --delete-config?
> > > > > > > > > > > >
> > > > > > > > > > > > best,
> > > > > > > > > > > > Colin
> > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > David
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, Feb 27, 2020 at 11:15 PM Aneel Nazareth <
> > > > > > > > > aneel@confluent.io>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > I've created a PR for a potential implementation of this:
> > > > > > > > > > > > > > https://github.com/apache/kafka/pull/8184 if we decide
> > > > > to
> > > > > > go
> > > > > > > > > ahead
> > > > > > > > > > > > with
> > > > > > > > > > > > > > this KIP.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, Feb 26, 2020 at 12:36 PM Aneel Nazareth <
> > > > > > > > > aneel@confluent.io>
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I'd like to discuss adding a new argument to
> > > > > > kafka-configs.sh
> > > > > > > > > > > > > > > (ConfigCommand.scala).
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Recently I've been working on some things that require
> > > > > > > > complex
> > > > > > > > > > > > > > > configurations. I've chosen to represent them as JSON
> > > > > > strings
> > > > > > > > > in my
> > > > > > > > > > > > > > > server.properties. This works well, and I'm able to
> > > > > > update
> > > > > > > > the
> > > > > > > > > > > > > > > configurations by editing server.properties and
> > > > > > restarting
> > > > > > > > the
> > > > > > > > > > > > broker.
> > > > > > > > > > > > > > I've
> > > > > > > > > > > > > > > added the ability to dynamically configure them, and
> > > > > that
> > > > > > > > > works well
> > > > > > > > > > > > > > using
> > > > > > > > > > > > > > > the AdminClient. However, when I try to update these
> > > > > > > > > configurations
> > > > > > > > > > > > using
> > > > > > > > > > > > > > > kafka-configs.sh, I run into a problem. My
> > > > > configurations
> > > > > > > > > contain
> > > > > > > > > > > > commas,
> > > > > > > > > > > > > > > and kafka-configs.sh tries to break them up into
> > > > > > key/value
> > > > > > > > > pairs at
> > > > > > > > > > > > the
> > > > > > > > > > > > > > > comma boundary.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I'd like to enable setting these configurations from
> > > > > the
> > > > > > > > > command
> > > > > > > > > > > > line, so
> > > > > > > > > > > > > > > I'm proposing that we add a new option to
> > > > > > kafka-configs.sh
> > > > > > > > that
> > > > > > > > > > > > takes a
> > > > > > > > > > > > > > > properties file.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I've created a KIP for this idea:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-574%3A+CLI+Dynamic+Configuration+with+file+input
> > > > > > > > > > > > > > > And a JIRA:
> > > > > > https://issues.apache.org/jira/browse/KAFKA-9612
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I'd appreciate your feedback on the proposal.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > Aneel
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> >
Re: [DISCUSS] KIP-574: CLI Dynamic Configuration with file input
Posted by Aneel Nazareth <an...@confluent.io>.
Thanks to David Jacot for his suggestions. Would it be possible for a
committer to have a look at this PR? Thanks!
https://github.com/apache/kafka/pull/8184
On Wed, Apr 1, 2020 at 12:30 PM Aneel Nazareth <an...@confluent.io> wrote:
>
> I have a PR that I think is ready for review here:
> https://github.com/apache/kafka/pull/8184
>
> Feedback would be most welcome!
>
> On Mon, Mar 30, 2020 at 6:57 PM Colin McCabe <cm...@apache.org> wrote:
> >
> > Just as a note about whether we should support "-" as a synonym for STDIN... I agree it's kind of inconsistent.
> >
> > It may not be that big of a deal to drop support for STDIN. A lot of UNIX shells make it easy to create a temporary file in scripts-- for example, you could use a "here document" ( https://en.wikipedia.org/wiki/Here_document )
> >
> > best,
> > Colin
> >
> >
> > On Fri, Mar 27, 2020, at 07:46, Aneel Nazareth wrote:
> > > Update: I have simplified the KIP down to just adding the single new
> > > --add-config-file option. Thanks for your input, everyone!
> > >
> > > On Thu, Mar 26, 2020 at 10:13 AM Aneel Nazareth <an...@confluent.io> wrote:
> > > >
> > > > Hi Kamal,
> > > >
> > > > Thanks for taking a look at this KIP.
> > > >
> > > > Unfortunately the user actually can't pass the arguments on the
> > > > command line using the existing --add-config option if the values are
> > > > complex structures that contain commas. --add-config assumes that
> > > > commas separate distinct configuration properties. There's a
> > > > workaround using square brackets ("[a,b,c]") for simple lists, but it
> > > > doesn't work for things like nested lists or JSON values.
> > > >
> > > > The motivation for allowing STDIN as well as files is to enable
> > > > grep/pipe workflows in scripts without creating a temporary file. I
> > > > don't know if such workflows will end up being common, and hopefully
> > > > someone with a complex enough use case to require it would also be
> > > > familiar with techniques for securely creating and cleaning up
> > > > temporary files.
> > > >
> > > > I'm okay with excluding the option to allow STDIN in the name of
> > > > consistency, if the consensus thinks that's wise. Anyone else have
> > > > opinions on this?
> > > >
> > > > On Thu, Mar 26, 2020 at 9:02 AM Kamal Chandraprakash
> > > > <ka...@gmail.com> wrote:
> > > > >
> > > > > Hi Colin,
> > > > >
> > > > > We should not support STDIN to maintain uniformity across scripts. If the
> > > > > user wants to pass the arguments in command line,
> > > > > they can always use the existing --add-config option.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Mar 26, 2020 at 7:20 PM David Jacot <dj...@confluent.io> wrote:
> > > > >
> > > > > > Rajini has made a good point. I don't feel strong for either ways but if
> > > > > > people
> > > > > > are confused by this, it is probably better without it.
> > > > > >
> > > > > > Best,
> > > > > > David
> > > > > >
> > > > > > On Thu, Mar 26, 2020 at 7:23 AM Colin McCabe <cm...@apache.org> wrote:
> > > > > >
> > > > > > > Hi Kamal,
> > > > > > >
> > > > > > > Are you suggesting that we not support STDIN here? I have mixed
> > > > > > feelings.
> > > > > > >
> > > > > > > I think the ideal solution would be to support "-" in these tools
> > > > > > whenever
> > > > > > > a file argument was expected. But that would be a bigger change than
> > > > > > what
> > > > > > > we're talking about here. Maybe you are right and we should keep it
> > > > > > simple
> > > > > > > for now.
> > > > > > >
> > > > > > > best,
> > > > > > > Colin
> > > > > > >
> > > > > > > On Wed, Mar 25, 2020, at 01:24, Kamal Chandraprakash wrote:
> > > > > > > > STDIN wasn't standard practice in other scripts like
> > > > > > > > kafka-console-consumer.sh, kafka-console-producer.sh and kafka-acls.sh
> > > > > > > > in which the props file is accepted via consumer.config /
> > > > > > > producer.config /
> > > > > > > > command-config parameter.
> > > > > > > >
> > > > > > > > Shouldn't we have to maintain the uniformity across scripts?
> > > > > > > >
> > > > > > > > On Mon, Mar 16, 2020 at 4:13 PM David Jacot <dj...@confluent.io>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Aneel,
> > > > > > > > >
> > > > > > > > > Thanks for the updated KIP. I have made a second pass over it and the
> > > > > > > > > KIP looks good to me.
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > David
> > > > > > > > >
> > > > > > > > > On Tue, Mar 10, 2020 at 9:39 PM Aneel Nazareth <an...@confluent.io>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > After reading a bit more about it in the Kubernetes case, I think
> > > > > > > it's
> > > > > > > > > > reasonable to do this and be explicit that we're ignoring the
> > > > > > value,
> > > > > > > > > > just deleting all keys that appear in the file.
> > > > > > > > > >
> > > > > > > > > > I've updated the KIP wiki page to reflect that:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-574%3A+CLI+Dynamic+Configuration+with+file+input
> > > > > > > > > >
> > > > > > > > > > And updated my sample PR:
> > > > > > > > > > https://github.com/apache/kafka/pull/8184
> > > > > > > > > >
> > > > > > > > > > If there are no further comments, I'll request a vote in a few
> > > > > > days.
> > > > > > > > > >
> > > > > > > > > > Thanks for the feedback!
> > > > > > > > > >
> > > > > > > > > > On Mon, Mar 9, 2020 at 1:24 PM Aneel Nazareth <an...@confluent.io>
> > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi David,
> > > > > > > > > > >
> > > > > > > > > > > Is the expected behavior that the keys are deleted without
> > > > > > > checking the
> > > > > > > > > > values?
> > > > > > > > > > >
> > > > > > > > > > > Let's say I had this file new.properties:
> > > > > > > > > > > a=1
> > > > > > > > > > > b=2
> > > > > > > > > > >
> > > > > > > > > > > And ran:
> > > > > > > > > > >
> > > > > > > > > > > bin/kafka-configs --bootstrap-server localhost:9092 \
> > > > > > > > > > > --entity-type brokers --entity-default \
> > > > > > > > > > > --alter --add-config-file new.properties
> > > > > > > > > > >
> > > > > > > > > > > It seems clear what should happen if I run this immediately:
> > > > > > > > > > >
> > > > > > > > > > > bin/kafka-configs --bootstrap-server localhost:9092 \
> > > > > > > > > > > --entity-type brokers --entity-default \
> > > > > > > > > > > --alter --delete-config-file new.properties
> > > > > > > > > > >
> > > > > > > > > > > (Namely that both a and b would now have no values in the config)
> > > > > > > > > > >
> > > > > > > > > > > But what if this were run in-between:
> > > > > > > > > > >
> > > > > > > > > > > bin/kafka-configs --bootstrap-server localhost:9092 \
> > > > > > > > > > > --entity-type brokers --entity-default \
> > > > > > > > > > > --alter --add-config a=3
> > > > > > > > > > >
> > > > > > > > > > > Would it be surprising if the key/value pair a=3 was deleted,
> > > > > > even
> > > > > > > > > > > though the config that is in the file is a=1? Or would that be
> > > > > > > > > > > expected?
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Mar 9, 2020 at 1:02 PM David Jacot <dj...@confluent.io>
> > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Colin,
> > > > > > > > > > > >
> > > > > > > > > > > > Yes, you're right. This is weird but convenient because you
> > > > > > don't
> > > > > > > > > have
> > > > > > > > > > to
> > > > > > > > > > > > duplicate
> > > > > > > > > > > > the "keys". I was thinking about the kubernetes API which
> > > > > > allows
> > > > > > > to
> > > > > > > > > > create
> > > > > > > > > > > > a Pod
> > > > > > > > > > > > based on a file and allows to delete it as well with the same
> > > > > > > file. I
> > > > > > > > > > have
> > > > > > > > > > > > always found
> > > > > > > > > > > > this convenient, especially when doing local tests.
> > > > > > > > > > > >
> > > > > > > > > > > > Best,
> > > > > > > > > > > > David
> > > > > > > > > > > >
> > > > > > > > > > > > On Mon, Mar 9, 2020 at 6:35 PM Colin McCabe <
> > > > > > cmccabe@apache.org>
> > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hi Aneel,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for the KIP. I like the idea.
> > > > > > > > > > > > >
> > > > > > > > > > > > > You mention that "input from STDIN can be used instead of a
> > > > > > > file on
> > > > > > > > > > > > > disk." The example given in the KIP seems to suggest that
> > > > > > the
> > > > > > > > > > command
> > > > > > > > > > > > > defaults to reading from STDIN if no argument is given to
> > > > > > > > > > --add-config-file.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I would argue against this particular command-line pattern.
> > > > > > > From
> > > > > > > > > the
> > > > > > > > > > > > > user's point of view, if they mess up and forget to supply an
> > > > > > > > > > argument, or
> > > > > > > > > > > > > for some reason the parser doesn't treat something like an
> > > > > > > > > argument,
> > > > > > > > > > the
> > > > > > > > > > > > > program will appear to hang in a confusing way.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Instead, it would be better to follow the traditional UNIX
> > > > > > > pattern
> > > > > > > > > > where a
> > > > > > > > > > > > > dash indicates that STDIN should be read. So
> > > > > > > "--add-config-file -"
> > > > > > > > > > would
> > > > > > > > > > > > > indicate that the program should read form STDIN. This would
> > > > > > > be
> > > > > > > > > > difficult
> > > > > > > > > > > > > to trigger accidentally, and more in line with the
> > > > > > traditional
> > > > > > > > > > conventions.
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Mon, Mar 9, 2020, at 08:47, David Jacot wrote:
> > > > > > > > > > > > > > I wonder if we should also add a `--delete-config-file` as
> > > > > > a
> > > > > > > > > > counterpart
> > > > > > > > > > > > > of
> > > > > > > > > > > > > > `--add-config-file`. It would be a bit weird to use a
> > > > > > > properties
> > > > > > > > > > file in
> > > > > > > > > > > > > > this case as the values are not necessary but it may be
> > > > > > > handy to
> > > > > > > > > > have the
> > > > > > > > > > > > > > possibility to remove the configurations which have been
> > > > > > set.
> > > > > > > > > Have
> > > > > > > > > > you
> > > > > > > > > > > > > > considered this?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi David,
> > > > > > > > > > > > >
> > > > > > > > > > > > > That's an interesting idea. However, I think it might be
> > > > > > > confusing
> > > > > > > > > > to
> > > > > > > > > > > > > users to supply a file, and then have the values supplied in
> > > > > > > that
> > > > > > > > > > file be
> > > > > > > > > > > > > ignored. Is there really a case where we need to do this (as
> > > > > > > > > > opposed to
> > > > > > > > > > > > > creating a file with blank values, or just passing the keys
> > > > > > to
> > > > > > > > > > > > > --delete-config?
> > > > > > > > > > > > >
> > > > > > > > > > > > > best,
> > > > > > > > > > > > > Colin
> > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > David
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Thu, Feb 27, 2020 at 11:15 PM Aneel Nazareth <
> > > > > > > > > > aneel@confluent.io>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > I've created a PR for a potential implementation of this:
> > > > > > > > > > > > > > > https://github.com/apache/kafka/pull/8184 if we decide
> > > > > > to
> > > > > > > go
> > > > > > > > > > ahead
> > > > > > > > > > > > > with
> > > > > > > > > > > > > > > this KIP.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Wed, Feb 26, 2020 at 12:36 PM Aneel Nazareth <
> > > > > > > > > > aneel@confluent.io>
> > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I'd like to discuss adding a new argument to
> > > > > > > kafka-configs.sh
> > > > > > > > > > > > > > > > (ConfigCommand.scala).
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Recently I've been working on some things that require
> > > > > > > > > complex
> > > > > > > > > > > > > > > > configurations. I've chosen to represent them as JSON
> > > > > > > strings
> > > > > > > > > > in my
> > > > > > > > > > > > > > > > server.properties. This works well, and I'm able to
> > > > > > > update
> > > > > > > > > the
> > > > > > > > > > > > > > > > configurations by editing server.properties and
> > > > > > > restarting
> > > > > > > > > the
> > > > > > > > > > > > > broker.
> > > > > > > > > > > > > > > I've
> > > > > > > > > > > > > > > > added the ability to dynamically configure them, and
> > > > > > that
> > > > > > > > > > works well
> > > > > > > > > > > > > > > using
> > > > > > > > > > > > > > > > the AdminClient. However, when I try to update these
> > > > > > > > > > configurations
> > > > > > > > > > > > > using
> > > > > > > > > > > > > > > > kafka-configs.sh, I run into a problem. My
> > > > > > configurations
> > > > > > > > > > contain
> > > > > > > > > > > > > commas,
> > > > > > > > > > > > > > > > and kafka-configs.sh tries to break them up into
> > > > > > > key/value
> > > > > > > > > > pairs at
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > comma boundary.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I'd like to enable setting these configurations from
> > > > > > the
> > > > > > > > > > command
> > > > > > > > > > > > > line, so
> > > > > > > > > > > > > > > > I'm proposing that we add a new option to
> > > > > > > kafka-configs.sh
> > > > > > > > > that
> > > > > > > > > > > > > takes a
> > > > > > > > > > > > > > > > properties file.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I've created a KIP for this idea:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-574%3A+CLI+Dynamic+Configuration+with+file+input
> > > > > > > > > > > > > > > > And a JIRA:
> > > > > > > https://issues.apache.org/jira/browse/KAFKA-9612
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > I'd appreciate your feedback on the proposal.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > > > Aneel
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > >