----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34415/#review84850 -----------------------------------------------------------
Thanks for the patch. A few comments below. clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java <https://reviews.apache.org/r/34415/#comment136261> Could we just remove this api and use the new one with versionId? clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java <https://reviews.apache.org/r/34415/#comment136260> Could we change all requests to use parse(buffer, versionId)? clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataRequest.java <https://reviews.apache.org/r/34415/#comment136259> It would be better if we can make this a complete sentence. Sth like "Version x for ConsumerMetadataResponse is not valid. Valid versions are 0 to 1". clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java <https://reviews.apache.org/r/34415/#comment136374> For responses, we actually don't have to maintain the compatibility of the all constructors. The reason is that a client always constructs a response from a struct. We can freely change the signature of other constructors since only the server will use them. - Jun Rao On May 19, 2015, 4:09 p.m., Andrii Biletskyi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34415/ > ----------------------------------------------------------- > > (Updated May 19, 2015, 4:09 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2195 > https://issues.apache.org/jira/browse/KAFKA-2195 > > > Repository: kafka > > > Description > ------- > > KAFKA-2195 - Add versionId to AbstractRequest.getErrorResponse and > AbstractRequest.getRequest > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java > 5e5308ec0e333179a9abbf4f3b750ea25ab57967 > > clients/src/main/java/org/apache/kafka/common/requests/ConsumerMetadataRequest.java > 04b90bfe62456a6739fe0299f1564dbd1850fe58 > clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java > 8686d83aa52e435c6adafbe9ff4bd1602281072a > > clients/src/main/java/org/apache/kafka/common/requests/HeartbeatRequest.java > 51d081fa296fd7c170a90a634d432067afcfe772 > > clients/src/main/java/org/apache/kafka/common/requests/JoinGroupRequest.java > 6795682258e6b329cc3caa245b950b4dbcf0cf45 > > clients/src/main/java/org/apache/kafka/common/requests/JoinGroupResponse.java > fd9c545c99058ad3fbe3b2c55ea8b6ea002f5a51 > > clients/src/main/java/org/apache/kafka/common/requests/ListOffsetRequest.java > 19267ee8aad5a2f5a84cecd6fc563f00329d5035 > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java > 7e0ce159a2ddd041fc06116038bd5831bbca278b > > clients/src/main/java/org/apache/kafka/common/requests/MetadataResponse.java > 44e2ce61899889601b6aee71fa7f7ddb4a65a255 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitRequest.java > 8bf6cbb79a92b0878096e099ec9169d21e6d7023 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchRequest.java > deec1fa480d5a5c5884a1c007b076aa64e902472 > clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java > fabeae3083a8ea55cdacbb9568f3847ccd85bab4 > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > e3cc1967e407b64cc734548c19e30de700b64ba8 > core/src/main/scala/kafka/network/RequestChannel.scala > 1d0024c8f0c2ab0efa6d8cfca6455877a6ed8026 > core/src/main/scala/kafka/server/KafkaApis.scala > 387e387998fc3a6c9cb585dab02b5f77b0381fbf > > Diff: https://reviews.apache.org/r/34415/diff/ > > > Testing > ------- > > > Thanks, > > Andrii Biletskyi > >
