Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread joel koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28481 --- Ship it! - joel koshy On Nov. 8, 2013, 1:07 a.m., Neha Narkhede w

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread joel koshy
> On Nov. 7, 2013, 6:29 p.m., joel koshy wrote: > > core/src/main/scala/kafka/admin/AdminUtils.scala, line 224 > > > > > > Not sure if this is required any more if we're doing validation earlier > > - since a validati

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread joel koshy
> On Nov. 7, 2013, 6:31 p.m., joel koshy wrote: > > core/src/main/scala/kafka/admin/TopicCommand.scala, line 153 > > > > > > Also, if we are adding the deleteConfig option we need not use the same > > regex and split.

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
> On Nov. 7, 2013, 6:31 p.m., joel koshy wrote: > > core/src/main/scala/kafka/admin/TopicCommand.scala, line 153 > > > > > > Also, if we are adding the deleteConfig option we need not use the same > > regex and split.

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
> On Nov. 7, 2013, 6:29 p.m., joel koshy wrote: > > core/src/main/scala/kafka/admin/AdminUtils.scala, line 224 > > > > > > Not sure if this is required any more if we're doing validation earlier > > - since a validati

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
--- 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

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Swapnil Ghike
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28440 --- core/src/main/scala/kafka/admin/TopicCommand.scala

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread joel koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28395 --- core/src/main/scala/kafka/admin/TopicCommand.scala

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread joel koshy
--- 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

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
> On Nov. 7, 2013, 5:57 p.m., joel koshy wrote: > > core/src/main/scala/kafka/admin/TopicCommand.scala, line 153 > > > > > > This convention should be fine, but personally would prefer an unconfig > > or del-config si

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
--- 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

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread joel koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28385 --- core/src/main/scala/kafka/admin/AdminUtils.scala

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Guozhang Wang
> On Nov. 6, 2013, 9:41 p.m., Guozhang Wang wrote: > > Topic config change as well as create-topic, add-partition, > > partition-reassignment and preferred leader election are all asynchronous > > in the sense that the admin command would return immediately and one has to > > check himself if

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/ --- (Updated Nov. 7, 2013, 5:50 p.m.) Review request for kafka. Bugs: KAFKA-1119

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
> On Nov. 6, 2013, 10:42 p.m., Swapnil Ghike wrote: > > It will help to add a --deleteConfig option. We can use it as --config > > config1=newVal --deleteConfig config2. > > Neha Narkhede wrote: > Like we discussed offline, deleteConfig complicates the ability to accept > configs easily in

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/ --- (Updated Nov. 7, 2013, 4:50 p.m.) Review request for kafka. Changes ---

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
> On Nov. 6, 2013, 9:41 p.m., Guozhang Wang wrote: > > Topic config change as well as create-topic, add-partition, > > partition-reassignment and preferred leader election are all asynchronous > > in the sense that the admin command would return immediately and one has to > > check himself if

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-07 Thread Neha Narkhede
> On Nov. 6, 2013, 10:42 p.m., Swapnil Ghike wrote: > > It will help to add a --deleteConfig option. We can use it as --config > > config1=newVal --deleteConfig config2. Like we discussed offline, deleteConfig complicates the ability to accept configs easily in bulk and apply those. - Neha

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-06 Thread Swapnil Ghike
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28327 --- It will help to add a --deleteConfig option. We can use it as --conf

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-06 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28319 --- Ship it! Topic config change as well as create-topic, add-partition

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-06 Thread Neha Narkhede
> On Nov. 6, 2013, 6:27 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/admin/AdminUtils.scala, line 219 > > > > > > Shall we use null instead of "" for deleted config values? Sometime in > > the future we ma

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-06 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/#review28283 --- core/src/main/scala/kafka/admin/AdminUtils.scala

Re: Review Request 15274: Patch for KAFKA-1119

2013-11-06 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15274/ --- (Updated Nov. 6, 2013, 6:13 p.m.) Review request for kafka. Bugs: KAFKA-1119