You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jun Rao <ju...@gmail.com> on 2015/07/07 19:22:35 UTC
Re: Review Request 35677: Patch for KAFKA-2288
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35677/#review90725
-----------------------------------------------------------
Ship it!
Thanks for the patch. +1. You can commit after addressing the following minor comments.
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala (lines 364 - 368)
<https://reviews.apache.org/r/35677/#comment143860>
Would it be better to override equal() and hashcode() of KafkaConfig?
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala (line 470)
<https://reviews.apache.org/r/35677/#comment143861>
Is the ZK connect value correct?
- Jun Rao
On June 22, 2015, 5:02 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35677/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 5:02 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2288
> https://issues.apache.org/jira/browse/KAFKA-2288
>
>
> Repository: kafka
>
>
> Description
> -------
>
> removed logging of topic overrides
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> core/src/main/scala/kafka/log/LogConfig.scala fc41132d2bf29439225ec581829eb479f98cc416
> core/src/test/scala/unit/kafka/log/LogConfigTest.scala 19dcb47f3f406b8d6c3668297450ab6b534e4471
> core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 98a5b042a710d3c1064b0379db1d152efc9eabee
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2428dbd7197a58cf4cad42ef82b385dab3a2b15e
>
> Diff: https://reviews.apache.org/r/35677/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gwen Shapira
>
>
Re: Review Request 35677: Patch for KAFKA-2288
Posted by Gwen Shapira <gs...@cloudera.com>.
> On July 7, 2015, 5:22 p.m., Jun Rao wrote:
> > Thanks for the patch. +1. You can commit after addressing the following minor comments.
I'm fixing these, but there was also some rebasing, so another look at the patch for sanity will be appreciated :)
> On July 7, 2015, 5:22 p.m., Jun Rao wrote:
> > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala, lines 364-368
> > <https://reviews.apache.org/r/35677/diff/2/?file=989813#file989813line364>
> >
> > Would it be better to override equal() and hashcode() of KafkaConfig?
Overrode in AbstractConfig. I consider two AbstractConfig classes equal if the originals Map is equal. Sounds reasonable?
> On July 7, 2015, 5:22 p.m., Jun Rao wrote:
> > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala, line 470
> > <https://reviews.apache.org/r/35677/diff/2/?file=989813#file989813line470>
> >
> > Is the ZK connect value correct?
Any string will be correct here, but I'll add a port too :)
- Gwen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35677/#review90725
-----------------------------------------------------------
On June 22, 2015, 5:02 p.m., Gwen Shapira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35677/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 5:02 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2288
> https://issues.apache.org/jira/browse/KAFKA-2288
>
>
> Repository: kafka
>
>
> Description
> -------
>
> removed logging of topic overrides
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java bae528d31516679bed88ee61b408f209f185a8cc
> core/src/main/scala/kafka/log/LogConfig.scala fc41132d2bf29439225ec581829eb479f98cc416
> core/src/test/scala/unit/kafka/log/LogConfigTest.scala 19dcb47f3f406b8d6c3668297450ab6b534e4471
> core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala 98a5b042a710d3c1064b0379db1d152efc9eabee
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 2428dbd7197a58cf4cad42ef82b385dab3a2b15e
>
> Diff: https://reviews.apache.org/r/35677/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gwen Shapira
>
>