You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Gwen Shapira <gs...@cloudera.com> on 2015/03/24 23:39:23 UTC

Review Request 32459: Patch for KAFKA-2044

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

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


Diffs
-----

  clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java 37aff6c0fd2ec2da8551aa74b166ca49b224ddd3 
  clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java 6943878116a97c02b758d273d93976019688830e 
  clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java 1ebc188742fd65e5e744003b4579324874fd81a9 
  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


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32459/
-----------------------------------------------------------

(Updated March 27, 2015, 4:22 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

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


fixed Metadata error response


merge with trunk


Diffs (updated)
-----

  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/MetadataResponse.java 36736ecb29de122e64fdb9e0cc042f7aaed27946 
  clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java b92f6705e2cce4c64a117166e621a5388fd249a6 
  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 61a767aaa195bd9cae61180c591a0dde8278d71e 
  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 c33e848256be6159a148319bf74bf1590f27cc06 
  core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala 4cb803f3e86dbc97f1749b2db9788fe9d376d49a 

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


Testing
-------


Thanks,

Gwen Shapira


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.

> On March 26, 2015, 3:54 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java, lines 49-64
> > <https://reviews.apache.org/r/32459/diff/6/?file=906393#file906393line49>
> >
> >     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.

Errr... yeah. That was a total hack, since I figured no one will use this until we migrate to using this class in core, and it can be fixed then. Anyway, rewriting this to something reasonable and uploading a rebased version.


- Gwen


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


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


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-------

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 (updated)
-----

  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


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32459/
-----------------------------------------------------------

(Updated March 26, 2015, 1:49 a.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

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


Diffs (updated)
-----

  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


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Jun Rao <ju...@gmail.com>.

> On March 26, 2015, 12:47 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java, line 53
> > <https://reviews.apache.org/r/32459/diff/4/?file=906206#file906206line53>
> >
> >     Cluster may not serialize properly with those nulls in PartitionInfo.
> 
> Gwen Shapira wrote:
>     Oh wow, great catch. Added tests and fixed the serialization issues.
> 
> Gwen Shapira wrote:
>     I'm hitting a style-check error for the test: /Users/gshapira/workspace/kafka/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java:20:1: Disallowed import - org.apache.kafka.common.errors.UnknownServerException.
>     
>     I need an exception to test "getErrorResponse()", any idea why I'm not allowed to import this here? both packages are in o.a.k.common...

Probably because of the following rule in checkstyle/import-control.xml?

                <subpackage name="requests">
                        <allow pkg="org.apache.kafka.common.protocol" />
                        <allow pkg="org.apache.kafka.common.network" />
                </subpackage>


- Jun


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


On March 26, 2015, 1:49 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, 1:49 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
> 
> 
> Diffs
> -----
> 
>   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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.

> On March 26, 2015, 12:47 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java, line 53
> > <https://reviews.apache.org/r/32459/diff/4/?file=906206#file906206line53>
> >
> >     Cluster may not serialize properly with those nulls in PartitionInfo.
> 
> Gwen Shapira wrote:
>     Oh wow, great catch. Added tests and fixed the serialization issues.

I'm hitting a style-check error for the test: /Users/gshapira/workspace/kafka/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java:20:1: Disallowed import - org.apache.kafka.common.errors.UnknownServerException.

I need an exception to test "getErrorResponse()", any idea why I'm not allowed to import this here? both packages are in o.a.k.common...


- Gwen


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


On March 26, 2015, 1:49 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, 1:49 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
> 
> 
> Diffs
> -----
> 
>   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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.

> On March 26, 2015, 12:47 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java, line 53
> > <https://reviews.apache.org/r/32459/diff/4/?file=906206#file906206line53>
> >
> >     Cluster may not serialize properly with those nulls in PartitionInfo.

Oh wow, great catch. Added tests and fixed the serialization issues.


- Gwen


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


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


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.

> On March 26, 2015, 12:47 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java, line 53
> > <https://reviews.apache.org/r/32459/diff/4/?file=906206#file906206line53>
> >
> >     Cluster may not serialize properly with those nulls in PartitionInfo.
> 
> Gwen Shapira wrote:
>     Oh wow, great catch. Added tests and fixed the serialization issues.
> 
> Gwen Shapira wrote:
>     I'm hitting a style-check error for the test: /Users/gshapira/workspace/kafka/clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java:20:1: Disallowed import - org.apache.kafka.common.errors.UnknownServerException.
>     
>     I need an exception to test "getErrorResponse()", any idea why I'm not allowed to import this here? both packages are in o.a.k.common...
> 
> Jun Rao wrote:
>     Probably because of the following rule in checkstyle/import-control.xml?
>     
>                     <subpackage name="requests">
>                             <allow pkg="org.apache.kafka.common.protocol" />
>                             <allow pkg="org.apache.kafka.common.network" />
>                     </subpackage>

Does it make sense to change the rule here?


- Gwen


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


On March 26, 2015, 1:49 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, 1:49 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
> 
> 
> Diffs
> -----
> 
>   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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-------

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 (updated)
-----

  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


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32459/
-----------------------------------------------------------

(Updated March 25, 2015, 11:48 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

support requests and responses using Common api in core modules (missing files)


added error handling and factory method for requests


KAFKA-2047; Move the stream creation into concurrent mirror maker threads; reviewed by Guozhang Wang


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 (updated)
-----

  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/main/scala/kafka/tools/MirrorMaker.scala 4f3c4c872e144195bb4b742b802fa3b931edb534 
  core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6 

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


Testing
-------


Thanks,

Gwen Shapira


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32459/
-----------------------------------------------------------

(Updated March 25, 2015, 11:48 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

support requests and responses using Common api in core modules (missing files)


added error handling and factory method for requests


KAFKA-2047; Move the stream creation into concurrent mirror maker threads; reviewed by Guozhang Wang


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into KAFKA-1927-v2


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java 37aff6c0fd2ec2da8551aa74b166ca49b224ddd3 
  clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java 6943878116a97c02b758d273d93976019688830e 
  clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java 1ebc188742fd65e5e744003b4579324874fd81a9 
  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/main/scala/kafka/tools/MirrorMaker.scala 4f3c4c872e144195bb4b742b802fa3b931edb534 
  core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala fba852afa1b2f46b61e2fd12c38c821ba04e9cc6 

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


Testing
-------


Thanks,

Gwen Shapira


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Gwen Shapira <gs...@cloudera.com>.

> On March 25, 2015, 9:43 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java, lines 46-54
> > <https://reviews.apache.org/r/32459/diff/1/?file=904768#file904768line46>
> >
> >     Instead of adding this method here, would it be better to add it to an abstract subclass AbstractRequest? All requests will now to a subclass of AbstractRequest and they will be forced to implement this method.

This means adding a handleError to all Requests in this patch (even though the method will not be used until the request is actually used within KafkaApis).
If you think its safer (and I agree it may be), I'll go ahead and do this.


- Gwen


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


On March 24, 2015, 10:39 p.m., Gwen Shapira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32459/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 10:39 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
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java 37aff6c0fd2ec2da8551aa74b166ca49b224ddd3 
>   clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java 6943878116a97c02b758d273d93976019688830e 
>   clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java 1ebc188742fd65e5e744003b4579324874fd81a9 
>   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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Jun Rao <ju...@gmail.com>.

> On March 25, 2015, 9:43 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java, lines 46-54
> > <https://reviews.apache.org/r/32459/diff/1/?file=904768#file904768line46>
> >
> >     Instead of adding this method here, would it be better to add it to an abstract subclass AbstractRequest? All requests will now to a subclass of AbstractRequest and they will be forced to implement this method.
> 
> Gwen Shapira wrote:
>     This means adding a handleError to all Requests in this patch (even though the method will not be used until the request is actually used within KafkaApis).
>     If you think its safer (and I agree it may be), I'll go ahead and do this.

Yes, it does mean that we will add this method to all existing requests. I think this makes the code a bit cleaner and safer.


- Jun


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


On March 24, 2015, 10:39 p.m., Gwen Shapira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32459/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 10:39 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
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java 37aff6c0fd2ec2da8551aa74b166ca49b224ddd3 
>   clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java 6943878116a97c02b758d273d93976019688830e 
>   clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java 1ebc188742fd65e5e744003b4579324874fd81a9 
>   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
> 
>


Re: Review Request 32459: Patch for KAFKA-2044

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32459/#review77805
-----------------------------------------------------------


Thanks for the patch. Looks good to me overall. A few comments below.


clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java
<https://reviews.apache.org/r/32459/#comment126087>

    Instead of adding this method here, would it be better to add it to an abstract subclass AbstractRequest? All requests will now to a subclass of AbstractRequest and they will be forced to implement this method.



core/src/main/scala/kafka/network/RequestChannel.scala
<https://reviews.apache.org/r/32459/#comment126088>

    Need identation.



core/src/main/scala/kafka/network/RequestChannel.scala
<https://reviews.apache.org/r/32459/#comment126090>

    Need identation and be consistent with the previous statement.



core/src/main/scala/kafka/network/RequestChannel.scala
<https://reviews.apache.org/r/32459/#comment126091>

    Ditto as the above.


- Jun Rao


On March 24, 2015, 10:39 p.m., Gwen Shapira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32459/
> -----------------------------------------------------------
> 
> (Updated March 24, 2015, 10:39 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
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java 37aff6c0fd2ec2da8551aa74b166ca49b224ddd3 
>   clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java 6943878116a97c02b758d273d93976019688830e 
>   clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java 1ebc188742fd65e5e744003b4579324874fd81a9 
>   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
> 
>