Re: Review Request 30126: Patch for KAFKA-1845

2015-03-04 Thread Andrii Biletskyi 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/
---

(Updated March 4, 2015, 11:12 p.m.)


Review request for kafka.


Bugs: KAFKA-1845
https://issues.apache.org/jira/browse/KAFKA-1845


Repository: kafka


Description (updated)
---

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


KAFKA-1845 - code review fixes, merge conflicts after rebase


KAFKA-1845 - rebase to trunk


Diffs (updated)
-

  clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
852a9b39a2f9cd71d176943be86531a43ede 
  core/src/main/scala/kafka/Kafka.scala 
77a49e12af6f869e63230162e9f87a7b0b12b610 
  core/src/main/scala/kafka/controller/KafkaController.scala 
e9b4dc62df3f139de8d4dc688e48ccc0a5513123 
  core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala 
4a31c7271c2d0a4b9e8b28be729340ecfa0696e5 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
14bf3216bae030331bdf76b3266ed0e73526c3de 
  core/src/main/scala/kafka/server/KafkaServer.scala 
8e3def9e9edaf49c0443a2e08cf37277d0f25306 
  core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 
6879e730282185bda3d6bc3659cb15af0672cecf 
  core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala 
5650b4a7b950b48af3e272947bfb5e271c4238c9 
  core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala 
e63558889272bc76551accdfd554bdafde2e0dd6 
  core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala 
d34ee3a40dcc8475e183435ad6842cd3d0a13ade 
  core/src/test/scala/integration/kafka/api/ProducerSendTest.scala 
8154a4210dc8dcceb26549c98bbbc4a1282a1c67 
  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/DeleteConsumerGroupTest.scala 
d530338728be41282925b3a62030d1f316b4f9c5 
  core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 
c8f336aa034ab5702c7644a669fd32c746512d29 
  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 
aeb7a19acaefabcc161c2ee6144a56d9a8999a81 
  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 
1a4be70a21fe799d4d71dd13e84968e40fb8ad92 
  core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala 
4ea0489c9fd36983fe190491a086b39413f3a9cd 
  core/src/test/scala/unit/kafka/metrics/MetricsTest.scala 
111e4a26c1efb6f7c151ca9217dbe107c27ab617 
  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

Re: Review Request 30126: Patch for KAFKA-1845

2015-02-08 Thread Andrii Biletskyi 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/
---

(Updated Feb. 8, 2015, 3:05 p.m.)


Review request for kafka.


Bugs: KAFKA-1845
https://issues.apache.org/jira/browse/KAFKA-1845


Repository: kafka


Description (updated)
---

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


KAFKA-1845 - code review fixes, merge conflicts after rebase


Diffs (updated)
-

  clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java 
852a9b39a2f9cd71d176943be86531a43ede 
  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/IntegrationTestHarness.scala 
5650b4a7b950b48af3e272947bfb5e271c4238c9 
  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 
aeb7a19acaefabcc161c2ee6144a56d9a8999a81 
  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

Re: Review Request 30126: Patch for KAFKA-1845

2015-02-04 Thread Andrii Biletskyi 


 On Jan. 22, 2015, 7:57 p.m., Gwen Shapira wrote:
  core/src/main/scala/kafka/server/KafkaConfig.scala, lines 743-758
  https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line743
 
  This looks fairly generic. Shouldn't it be somewhere where it can be 
  reused by other Config classes? Perhaps in ConfigDef or something similar?

I've fixed the function slightly - valid predicate is needless here (it was 
ported from VerifiableProperties), so it's just really Try[] arround already 
utility function Utils.parseCsvMap(?str) . I doubt it worth moving simple 
try-catch to separate function now... Hope this resolves.


- Andrii


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69224
---


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

Re: Review Request 30126: Patch for KAFKA-1845

2015-02-04 Thread Andrii Biletskyi 


 On Feb. 3, 2015, 2:06 p.m., Jeff Holoman wrote:
  core/src/main/scala/kafka/server/KafkaConfig.scala, line 834
  https://reviews.apache.org/r/30126/diff/1/?file=828512#file828512line834
 
  Is this is the right way to do this rather than having these in 
  Validators?

Some of the broker settings validations are quite intricate - they involve 
other settings to be compared (e.g. replicaFetchWaitMaxMs = 
replicaSocketTimeoutMs), which we can't do inside ConfigDef.define.


- Andrii


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review70748
---


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

Re: Review Request 30126: Patch for KAFKA-1845

2015-02-03 Thread Joe Stein 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review70747
---



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116124

LOW



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116125

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116126

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116127

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116128

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116147

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116131

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116134

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116132

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116133

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116135

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116136

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116137

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116138

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116139

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116140

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116141

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116142

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116143

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116144

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116145

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116146

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116148

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116149

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116150

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116152

MEDIUM



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116153

MEDIUM


- Joe Stein


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

Re: Review Request 30126: Patch for KAFKA-1845

2015-02-03 Thread Jeff Holoman 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review70748
---



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment116129

Is this is the right way to do this rather than having these in Validators?


- Jeff Holoman


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

Re: Review Request 30126: Patch for KAFKA-1845

2015-02-03 Thread Andrii Biletskyi 


 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: MapString, 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

Re: Review Request 30126: Patch for KAFKA-1845

2015-01-22 Thread Gwen Shapira 


 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)

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.


- Gwen


---
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

Re: Review Request 30126: Patch for KAFKA-1845

2015-01-21 Thread Eric Olander 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30126/#review69063
---



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment113635

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.



core/src/main/scala/kafka/server/KafkaConfig.scala
https://reviews.apache.org/r/30126/#comment113634

Maybe some helper functions could help with this code:

def stringProp(prop: String) = parsed.get(prop).asInstanceOf[String]

then:
zkConnect = stringProp(ZkConnectProp)


- Eric Olander


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