> On Jan. 21, 2015, 10:55 p.m., Eric Olander wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 130
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line130>
> >
> > It seems that by convention there is a ...Prop and a ...Doc constant,
> > but nothing enforces that. Maybe have
> > val ZKConnect = ("zookeeper.connect", "Zookeeper host string")
> > so it is more apparent that these two values are needed and related. A
> > utility class would be better than using a Tuple2, but that's the general
> > idea.
you mean smth like
```
case class Key(prop: String, doc: String)
```
?
I can even do
```
val (ZkConnectProp, ZkConnectDoc) = ("zookeeper.connect", "Zookeeper host
string")
```
I don't mind, it's just ProducerConfig and LogConfig then won't follow this
approach and again we'll end up with not uniform config implementations...
Let's wait if others support this since it's a big change and I'd rather do it
once :)
> On Jan. 21, 2015, 10:55 p.m., Eric Olander wrote:
> > core/src/main/scala/kafka/server/KafkaConfig.scala, line 475
> > <https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line475>
> >
> > Maybe some helper functions could help with this code:
> >
> > def stringProp(prop: String) = parsed.get(prop).asInstanceOf[String]
> >
> > then:
> > zkConnect = stringProp(ZkConnectProp)
>
> Gwen Shapira wrote:
> I like this suggestion. I'd add it to ConfigDef or something similar, so
> all Config classes can enjoy this.
> This can be a follow-up patch, since we are already using the
> parsed.get.asInstanceOf pattern everywhere, so the fix is not related to this
> patch specifically.
Yes, good point!
Just one thing: if we add it to ConfigDef java class (which I like too), we
won't be able to use implicits so the utility function will be less gracefull:
```
String stringProp(parsed: Map<String, Object>, prop: String)
```
and sample usage:
```
zkConnect = stringProp(parsed, ZkConnectProp)
```
- Andrii
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69063
-----------------------------------------------------------
On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30126/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 5:49 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1845
> https://issues.apache.org/jira/browse/KAFKA-1845
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1845 - Fixed merge conflicts, ported added configs to KafkaConfig
>
>
> KAFKA-1845 - KafkaConfig to ConfigDef: moved validateValues so it's called on
> instantiating KafkaConfig
>
>
> KAFKA-1845 - KafkaConfig to ConfigDef: MaxConnectionsPerIpOverrides refactored
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java
> 98cb79b701918eca3f6ca9823b6c7b7c97b3ecec
> core/src/main/scala/kafka/Kafka.scala
> 77a49e12af6f869e63230162e9f87a7b0b12b610
> core/src/main/scala/kafka/controller/KafkaController.scala
> 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf
> core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala
> 4a31c7271c2d0a4b9e8b28be729340ecfa0696e5
> core/src/main/scala/kafka/server/KafkaConfig.scala
> 6d74983472249eac808d361344c58cc2858ec971
> core/src/main/scala/kafka/server/KafkaServer.scala
> 89200da30a04943f0b9befe84ab17e62b747c8c4
> core/src/main/scala/kafka/server/ReplicaFetcherThread.scala
> 6879e730282185bda3d6bc3659cb15af0672cecf
> core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala
> e63558889272bc76551accdfd554bdafde2e0dd6
> core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala
> 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab
> core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
> b15237b76def3b234924280fa3fdb25dbb0cc0dc
> core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala
> 1bf2667f47853585bc33ffb3e28256ec5f24ae84
> core/src/test/scala/unit/kafka/admin/AdminTest.scala
> e28979827110dfbbb92fe5b152e7f1cc973de400
> core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
> 33c27678bf8ae8feebcbcdaa4b90a1963157b4a5
> core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala
> c0355cc0135c6af2e346b4715659353a31723b86
>
> core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala
> a17e8532c44aadf84b8da3a57bcc797a848b5020
> core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala
> 95303e098d40cd790fb370e9b5a47d20860a6da3
> core/src/test/scala/unit/kafka/integration/FetcherTest.scala
> 25845abbcad2e79f56f729e59239b738d3ddbc9d
> core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala
> a5386a03b62956bc440b40783247c8cdf7432315
> core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala
> eab4b5f619015af42e4554660eafb5208e72ea33
> core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala
> 35dc071b1056e775326981573c9618d8046e601d
> core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala
> ba3bcdcd1de9843e75e5395dff2fc31b39a5a9d5
>
> core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala
> d6248b09bb0f86ee7d3bd0ebce5b99135491453b
> core/src/test/scala/unit/kafka/log/LogTest.scala
> c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e
> core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala
> 4ea0489c9fd36983fe190491a086b39413f3a9cd
> core/src/test/scala/unit/kafka/metrics/MetricsTest.scala
> 3cf23b3d6d4460535b90cfb36281714788fc681c
> core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala
> 1db6ac329f7b54e600802c8a623f80d159d4e69b
> core/src/test/scala/unit/kafka/producer/ProducerTest.scala
> ce65dab4910d9182e6774f6ef1a7f45561ec0c23
> core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala
> d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb
> core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala
> f0c4a56b61b4f081cf4bee799c6e9c523ff45e19
> core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala
> ad121169a5e80ebe1d311b95b219841ed69388e2
> core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala
> 8913fc1d59f717c6b3ed12c8362080fb5698986b
> core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala
> a703d2715048c5602635127451593903f8d20576
> core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala
> PRE-CREATION
> core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
> 82dce80d553957d8b5776a9e140c346d4e07f766
> core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala
> c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e
> core/src/test/scala/unit/kafka/server/LogOffsetTest.scala
> c06ee756bf0fe07e5d3c92823a476c960b37afd6
> core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala
> d5d351c4f25933da0ba776a6a89a989f1ca6a902
> core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala
> 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5
> core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala
> da4bafc1e2a94a436efe395aab1888fc21e55748
> core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala
> faa907131ed0aa94a7eacb78c1ffb576062be87a
> core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala
> cf2dd9455a9198b7215468df9f29568e91b31850
> core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala
> ba1e48e4300c9fb32e36e7266cb05294f2a481e5
> core/src/test/scala/unit/kafka/server/ServerStartupTest.scala
> 8fe7cd496f74ae1031019c2ca9ca013302294d9b
> core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala
> ccf5e2e36260b2484181b81d1b06e81de972674b
> core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala
> 84e08557de5acdcf0a98b192feac72836ea359b8
>
> Diff: https://reviews.apache.org/r/30126/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrii Biletskyi
>
>
