> On Nov. 7, 2013, 6:31 p.m., joel koshy wrote: > > core/src/main/scala/kafka/admin/TopicCommand.scala, line 153 > > <https://reviews.apache.org/r/15274/diff/4/?file=380482#file380482line153> > > > > Also, if we are adding the deleteConfig option we need not use the same > > regex and split. i.e., we expect only one argument. > > Neha Narkhede wrote: > That's true but the most common mistake while using this tool could be > to use deleteConfig like config. So I included the extra check in order to > give a relevant error message.
Ok - although I think the config validation would catch that if it is used like config (since there would be whitespace in the option value). - joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28395 ----------------------------------------------------------- On Nov. 8, 2013, 1:07 a.m., Neha Narkhede wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15274/ > ----------------------------------------------------------- > > (Updated Nov. 8, 2013, 1:07 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1119 > https://issues.apache.org/jira/browse/KAFKA-1119 > > > Repository: kafka > > > Description > ------- > > Improved the patch to simplify the usage of the internal changeTopicConfigs > API. It now includes only the final list of configs to be applied to a topic. > Any additions/deletions should be done prior to invoking this API > > > Addressed Joel's review comments > > > Add an explicit --deleteConfig option > > > more cleanup > > > Code cleanup > > > Fixed bug that now allows removing all the topic overrides correctly and > falling back to the defaults > > > 1. Change the --config to diff the previous configs 2. Add the ability to > remove per topic overrides if config is specified without a value > > > Diffs > ----- > > core/src/main/scala/kafka/admin/AdminUtils.scala > 8107a64cf1ef1cac763e152bae9f835411c9d3f3 > core/src/main/scala/kafka/admin/TopicCommand.scala > 56f3177e28a34df0ace1d192aef0060cb5e235df > core/src/main/scala/kafka/log/LogConfig.scala > 51ec796e9e6a10b76daefbd9aea02121fc1d573a > core/src/main/scala/kafka/server/TopicConfigManager.scala > 56cae58f2a216dcba88b2656fd4a490f11461270 > > Diff: https://reviews.apache.org/r/15274/diff/ > > > Testing > ------- > > Locally tested - 1. Adding new config 2. Adding new invalid config 3. > Deleting config 4. Deleting all config overrides > > > Thanks, > > Neha Narkhede > >
