----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32459/#review77832 -----------------------------------------------------------
Thanks for the new patch. A few comments below. Perhaps we can add a unit test to make sure that all error responses can be serialized properly. clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataRequest.java <https://reviews.apache.org/r/32459/#comment126145> It seems that ConsumerMetadataResponse won't work with a null broker. So we will have to create a fake broker as in ConsumerMetadataResponse.scala. clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java <https://reviews.apache.org/r/32459/#comment126146> Perhaps we can define -l and an empty ByteBuffer as a constant. clients/src/main/java/org/apache/kafka/common/requests/ListOffsetRequest.java <https://reviews.apache.org/r/32459/#comment126147> ListOffsetResponse.PartitionData doesn't seem to work with a null offset list. Probably passing in an empty list? clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java <https://reviews.apache.org/r/32459/#comment126148> Cluster may not serialize properly with those nulls in PartitionInfo. - Jun Rao On March 25, 2015, 11:53 p.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32459/ > ----------------------------------------------------------- > > (Updated March 25, 2015, 11:53 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2044 > https://issues.apache.org/jira/browse/KAFKA-2044 > > > Repository: kafka > > > Description > ------- > > support requests and responses using Common api in core modules (missing > files) > > > added error handling and factory method for requests > > > Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into > KAFKA-1927-v2 > > > made getErrorResponse required for requests by adding another abstract class > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/PartitionInfo.java > 321da8afc73941292f743e1c22fc3788df3d12dd > clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataRequest.java > 1651e75dedf32931eeff75f3ae6ef23db37acdc3 > clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java > 721e7d3f53247f5ae1ea4315fb3c466a94880b59 > > clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java > 6943878116a97c02b758d273d93976019688830e > > clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java > 1ebc188742fd65e5e744003b4579324874fd81a9 > > clients/src/main/java/org/apache/kafka/common/requests/ListOffsetRequest.java > e5dc92e9bb2aa5e291a99a67422ba3b0b80b31f7 > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java > 5d5f52c644e9ba3e9571c48e3e06b62edbb07fb5 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java > 94e9d376235b3288836807d8e8d2547b3743aad5 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchRequest.java > 16c807c01628b9408dcf20ca946373927246f7b0 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchResponse.java > f10c2463b53e157bc9f7ac3f017682fb3d1ace0e > clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java > 995f89f25b621484ddc3f3e4779ab7446a20124f > clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java > 4b67f7025fb613344ad65903f7bc8e3f61b165b4 > core/src/main/scala/kafka/api/HeartbeatRequestAndHeader.scala > f168d9fc99ce51b8b41b7f7db2a06f371b1a44e5 > core/src/main/scala/kafka/api/HeartbeatResponseAndHeader.scala > 9a71faae3138af1b4fb48125db619ddc3ad13c5a > core/src/main/scala/kafka/api/JoinGroupRequestAndHeader.scala > 3651e8603dd0ed0d2ea059786c68cf0722aa094b > core/src/main/scala/kafka/api/JoinGroupResponseAndHeader.scala > d0f07e0cbbdacf9ff8287e901ecabde3921bbab3 > core/src/main/scala/kafka/api/RequestKeys.scala > c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 > core/src/main/scala/kafka/network/BoundedByteBufferSend.scala > 55ecac285e00abf38d7131368bb46b4c4010dc87 > core/src/main/scala/kafka/network/RequestChannel.scala > 7b1db3dbbb2c0676f166890f566c14aa248467ab > core/src/main/scala/kafka/server/KafkaApis.scala > 35af98f0bc1b6a50bd1d97a30147593f8c6a422d > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > fba852afa1b2f46b61e2fd12c38c821ba04e9cc6 > > Diff: https://reviews.apache.org/r/32459/diff/ > > > Testing > ------- > > > Thanks, > > Gwen Shapira > >
