> On June 11, 2015, 11:22 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, lines 964-965 > > <https://reviews.apache.org/r/35347/diff/1/?file=982452#file982452line964> > > > > Is this castable since props is a map? Also, toProps is really just > > used by LogConfig which converts Properties back to a map. Perhaps we don't > > even need this method and just let LogConfig access > > KafkaConfig.originals(). > > > > KafkaConfig.getMetricClasses() uses toProps, but is never used. We can > > just remove that method. > > > > Also, perhaps it's useful and convenient to turn LogConfig into an > > AbstractConfig too.
I agree that we don't need to expose toProps since we can let other classes access originals directly. Just note that the current design allows for users to pass arbitrary properties to reporters by placing them in server.properties (since we pass original properties along when configuring reporters in getMetricClasses) - this is a critical feature and why getMetricClasses used to call toProps and will now call originals. - Gwen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35347/#review87643 ----------------------------------------------------------- On June 11, 2015, 6:09 a.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35347/ > ----------------------------------------------------------- > > (Updated June 11, 2015, 6:09 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2249 > https://issues.apache.org/jira/browse/KAFKA-2249 > > > Repository: kafka > > > Description > ------- > > modified KafkaConfig to implement AbstractConfig. This resulted in somewhat > cleaner code, and we preserve the original Properties for use by > MetricReporter > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > c4fa058692f50abb4f47bd344119d805c60123f5 > core/src/main/scala/kafka/controller/KafkaController.scala > 69bba243a9a511cc5292b43da0cc48e421a428b0 > core/src/main/scala/kafka/server/KafkaApis.scala > d63bc18d795a6f0e6994538ca55a9a46f7fb8ffd > core/src/main/scala/kafka/server/KafkaConfig.scala > 2d75186a110075e0c322db4b9f7a8c964a7a3e88 > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala > b31b432a226ba79546dd22ef1d2acbb439c2e9a3 > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > ace6321b36d809946554d205bc926c9c76a43bd6 > > Diff: https://reviews.apache.org/r/35347/diff/ > > > Testing > ------- > > > Thanks, > > Gwen Shapira > >
