----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32459/#review77907 -----------------------------------------------------------
Thanks for the new patch. Looks good overall. Just a couple of more comments. clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java <https://reviews.apache.org/r/32459/#comment126236> For the error response for MetadataRequest, we want to create one such that there is an error code per topic, but the partition list for each topic is empty. We probably need a new constructor in MetadataResponse to do that. clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java <https://reviews.apache.org/r/32459/#comment126238> requestList probably should be named requestOrResponseList. - Jun Rao On March 26, 2015, 2:20 a.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32459/ > ----------------------------------------------------------- > > (Updated March 26, 2015, 2:20 a.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 > > > added serialization tests for error responses and fixed related issues > > > Fix checkstyle complaint > > > Diffs > ----- > > checkstyle/import-control.xml cca4b38ec766028a604f88a1c63228e40df24573 > clients/src/main/java/org/apache/kafka/common/Node.java > 88c3b2425e42d9fc9c716a8e093d2ff1a12e28dd > 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/FetchResponse.java > f020aaa05525153f39dfda187f0c8174f83a6b95 > > 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 > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > 13237fd72da5448a3d596b882fef141f336f827d > 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 > >
