Re: Review Request 33383: Patch for KAFKA-1595

2015-04-21 Thread Ismael Juma

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

(Updated April 21, 2015, 7:03 a.m.)


Review request for kafka.


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


Repository: kafka


Description
---

Use the same `scalatest` version for all Scala
versions and remove unused code.

Introduce `testJsonParse`

Simple test that shows existing behaviour.

KAFKA-1595; Remove deprecated and slower scala JSON parser from 
kafka.consumer.TopicCount

A combination of spray-json's AST combined with jawn's parser are used as the 
replacement.
Note that both libraries have no dependencies and are relatively simple. We use 
`jawn` for
its performance, but it could be dropped by changing one line in 
`Json.parseFull`.

An attempt has been made to maintain the existing behaviour regarding when 
exceptions
are thrown. There are a number of cases where `DeserializationException` will 
be thrown
instead of `ClassCastException`, however. It is expected that users would not 
try to catch
`ClassCastException`.

Minor clean-ups in `Json.encode`


Diffs
-

  README.md 946ec62cc71df93c905c5f35caf5cdb9c78e5c10 
  build.gradle 4775ee46c480eab7b8250e61ba1705d00f72a6aa 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
eee80f9c2c12da8e4879e96785f3b75a8ff7d1cd 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala 
1c3b3802ac221d570e7610458e50518b4499e7ed 
  core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
3b3cd67d890e05c00d2a36a577f940347a0d387a 
  core/src/main/scala/kafka/cluster/Broker.scala 
79e16c167f67cfdef8a90212bc1c7607f989d102 
  core/src/main/scala/kafka/consumer/TopicCount.scala 
6994c8e89055b0bb300da6346c058c8fbbea2c29 
  core/src/main/scala/kafka/controller/KafkaController.scala 
3a09377611b48198c4c3cd1a118fc12eda0543d4 
  core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala 
d2bac85e16a247b1326f63619711fb0bbbd2e82a 
  core/src/main/scala/kafka/utils/Json.scala 
d1102844748f2e88f79932281fe95583a57d2d16 
  core/src/main/scala/kafka/utils/ReplicationUtils.scala 
60687332b4c9bee4d4c0851314cfb4b02d5d3489 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
5685a1eddb218baee617161f269cd1aee67bab9f 
  core/src/test/scala/unit/kafka/utils/JsonTest.scala 
93550e8f24071f88eb1ea5b41373efee27e4b8b7 

Diff: https://reviews.apache.org/r/33383/diff/


Testing (updated)
---

`testAll` succeeded eventually (it seems like some tests that rely on timings 
can sometimes fail).


Thanks,

Ismael Juma



Review Request 33383: Patch for KAFKA-1595

2015-04-20 Thread Ismael Juma

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

Review request for kafka.


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


Repository: kafka


Description
---

Use the same `scalatest` version for all Scala
versions and remove unused code.

Introduce `testJsonParse`

Simple test that shows existing behaviour.

KAFKA-1595; Remove deprecated and slower scala JSON parser from 
kafka.consumer.TopicCount

A combination of spray-json's AST combined with jawn's parser are used as the 
replacement.
Note that both libraries have no dependencies and are relatively simple. We use 
`jawn` for
its performance, but it could be dropped by changing one line in 
`Json.parseFull`.

An attempt has been made to maintain the existing behaviour regarding when 
exceptions
are thrown. There are a number of cases where `DeserializationException` will 
be thrown
instead of `ClassCastException`, however. It is expected that users would not 
try to catch
`ClassCastException`.

Minor clean-ups in `Json.encode`


Diffs
-

  README.md 946ec62cc71df93c905c5f35caf5cdb9c78e5c10 
  build.gradle 4775ee46c480eab7b8250e61ba1705d00f72a6aa 
  core/src/main/scala/kafka/admin/AdminUtils.scala 
eee80f9c2c12da8e4879e96785f3b75a8ff7d1cd 
  core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala 
1c3b3802ac221d570e7610458e50518b4499e7ed 
  core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala 
3b3cd67d890e05c00d2a36a577f940347a0d387a 
  core/src/main/scala/kafka/cluster/Broker.scala 
79e16c167f67cfdef8a90212bc1c7607f989d102 
  core/src/main/scala/kafka/consumer/TopicCount.scala 
6994c8e89055b0bb300da6346c058c8fbbea2c29 
  core/src/main/scala/kafka/controller/KafkaController.scala 
3a09377611b48198c4c3cd1a118fc12eda0543d4 
  core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala 
d2bac85e16a247b1326f63619711fb0bbbd2e82a 
  core/src/main/scala/kafka/utils/Json.scala 
d1102844748f2e88f79932281fe95583a57d2d16 
  core/src/main/scala/kafka/utils/ReplicationUtils.scala 
60687332b4c9bee4d4c0851314cfb4b02d5d3489 
  core/src/main/scala/kafka/utils/ZkUtils.scala 
5685a1eddb218baee617161f269cd1aee67bab9f 
  core/src/test/scala/unit/kafka/utils/JsonTest.scala 
93550e8f24071f88eb1ea5b41373efee27e4b8b7 

Diff: https://reviews.apache.org/r/33383/diff/


Testing
---


Thanks,

Ismael Juma