-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15274/#review28393
-----------------------------------------------------------



core/src/main/scala/kafka/admin/AdminUtils.scala
<https://reviews.apache.org/r/15274/#comment55224>

    Not sure if this is required any more if we're doing validation earlier - 
since a validation failure would be fatal.



core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/15274/#comment55223>

    This shouldn't be called for configsToBeDeleted right? - i.e., validation 
could fail for deleted configs which have no associated value.


- joel koshy


On Nov. 7, 2013, 6:17 p.m., Neha Narkhede wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15274/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2013, 6:17 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1119
>     https://issues.apache.org/jira/browse/KAFKA-1119
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> 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
> 
>

Reply via email to