kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aneel Nazareth <an...@confluent.io>
Subject Re: [DISCUSS] KIP-574: CLI Dynamic Configuration with file input
Date Tue, 10 Mar 2020 20:39:14 GMT
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 <aneel@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 <djacot@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
> > > > > >
> > > > >
> > > >
> > >

Mime
View raw message