> 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. > > Gwen Shapira wrote: > 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.
Turning LogConfig into an AbstractConfig should have been its own JIRA. The patch is now quite large since I had to modify creation of LogConfig in a bunch of tests. - Gwen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35347/#review87643 ----------------------------------------------------------- On June 18, 2015, 12:35 a.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35347/ > ----------------------------------------------------------- > > (Updated June 18, 2015, 12:35 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2249 > https://issues.apache.org/jira/browse/KAFKA-2249 > > > Repository: kafka > > > Description > ------- > > Moved LogConfig to implement AbstractConfig too. This means modifying most > Log tests, and some changes to defaults > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > c4fa058692f50abb4f47bd344119d805c60123f5 > core/src/main/scala/kafka/cluster/Partition.scala > 730a232482fdf77be5704cdf5941cfab3828db88 > core/src/main/scala/kafka/controller/KafkaController.scala > 69bba243a9a511cc5292b43da0cc48e421a428b0 > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala > 3b15ab4eef22c6f50a7483e99a6af40fb55aca9f > core/src/main/scala/kafka/log/LogConfig.scala > f64fd79ee4cdd5ad15cd9b14fe7247464cde1e94 > core/src/main/scala/kafka/log/LogManager.scala > e781ebac2677ebb22e0c1fef0cf7e5ad57c74ea4 > core/src/main/scala/kafka/server/KafkaApis.scala > c7debe458ce9d80024b3f8544c92ebe3e14159dc > core/src/main/scala/kafka/server/KafkaConfig.scala > 2d75186a110075e0c322db4b9f7a8c964a7a3e88 > core/src/main/scala/kafka/server/KafkaServer.scala > b320ce9f6a12c0ee392e91beb82e8804d167f9f4 > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala > 181cbc16b3780ffa77966cbc26337d2c39be9a72 > core/src/main/scala/kafka/server/TopicConfigManager.scala > b675a7e45ea4f4179f8b15fe221fd988aff13aa0 > core/src/main/scala/kafka/utils/CoreUtils.scala > d0a8fa701564b4c13b3cd6501e1b6218d77e8e06 > core/src/test/scala/other/kafka/StressTestLog.scala > c0e248d669c7bd653f512af7f72d895c38772f83 > core/src/test/scala/other/kafka/TestLinearWriteSpeed.scala > 3034c4f9b0d026e25ce045689d9a9f99a59a10ec > core/src/test/scala/unit/kafka/log/BrokerCompressionTest.scala > 375555f0684bbd0bfaf64b765ce04a928e257f0a > core/src/test/scala/unit/kafka/log/CleanerTest.scala > 8b8249a35322a60ca94cb385a6cad25943dd1cc9 > core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala > 471ddff9bff1bdfa277c071e59e5c6b749b9c74f > core/src/test/scala/unit/kafka/log/LogConfigTest.scala > 3fd5a53f9b0edc0a7a169a185cd3041ea1ae7658 > core/src/test/scala/unit/kafka/log/LogManagerTest.scala > 01dfbc4f8d21f6905327cd4ed6c61d657adc0143 > core/src/test/scala/unit/kafka/log/LogTest.scala > 8e095d652851f05365e1d3bbe3e9e1c3345b7a40 > core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala > 7877f6ca1845c2edbf96d4a9783a07a552db8f07 > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > c487f361949b2ca2b6d1b5e2c7fb9ba83c8e53c1 > > Diff: https://reviews.apache.org/r/35347/diff/ > > > Testing > ------- > > > Thanks, > > Gwen Shapira > >
