> On Aug. 10, 2015, 3:47 p.m., Grant Henke wrote: > > My apologies for not looking at this sooner, or suggesting this sooner. > > Given that this code change and scope is fairly large, would it be too much > > work to break out the patches & reviews by each new protocol message? Then > > reviews can be more accessable, focus on one thing at a time, and the > > simple ones can get done much quicker. I am thinking starting with > > CreateTopic, then DeleteTopic, then AlterTopic may makes sense. If you > > disagree feel free to say so and I will review this patch as is.
Hey Grant. Not sure this will work well. First of all this patch is already a part of one bigger feature KIP-4 (which we decided to split into patches :)). Secondly and most importantly, logic for handling all 3 requests relies on some base code. This means we will have to extract that common code (exceptions, some utils other stuff) first and probably submit it as a separate patch, which I'd prefer not to do. But I'm very open to suggestions if it speeds up the review process. - Andrii ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34766/#review94749 ----------------------------------------------------------- On June 30, 2015, 1:59 p.m., Andrii Biletskyi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34766/ > ----------------------------------------------------------- > > (Updated June 30, 2015, 1:59 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2229 > https://issues.apache.org/jira/browse/KAFKA-2229 > > > Repository: kafka > > > Description > ------- > > KIP-4 Phase 1 Rebase > > > Diffs > ----- > > checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 > clients/src/main/java/org/apache/kafka/common/ConfigEntry.java PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/PartitionReplicaAssignment.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/TopicConfigInfo.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java > b39e9bb53dd51f1ce931691e23aabc9ebaebf1e7 > clients/src/main/java/org/apache/kafka/common/protocol/Errors.java > 5b898c8f8ad5d0469f469b600c4b2eb13d1fc662 > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 3dc8b015afd2347a41c9a9dbc02b8e367da5f75f > clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java > 5d3d52859587ce0981d702f04042b0f6e1bc3704 > > clients/src/main/java/org/apache/kafka/common/requests/admin/AlterTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/AlterTopicResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/CreateTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/CreateTopicResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DeleteTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DeleteTopicResponse.java > PRE-CREATION > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > 8b2aca85fa738180e5420985fddc39a4bf9681ea > core/src/main/scala/kafka/admin/AdminUtils.scala > f06edf41c732a7b794e496d0048b0ce6f897e72b > core/src/main/scala/kafka/api/RequestKeys.scala > 155cb650e9cffe2c950326cfc25b1480cda819db > core/src/main/scala/kafka/common/InvalidPartitionsException.scala > PRE-CREATION > core/src/main/scala/kafka/common/InvalidReplicaAssignmentException.scala > PRE-CREATION > core/src/main/scala/kafka/common/InvalidReplicationFactorException.scala > PRE-CREATION > core/src/main/scala/kafka/common/InvalidTopicConfigurationException.scala > PRE-CREATION > > core/src/main/scala/kafka/common/ReassignPartitionsInProgressException.scala > PRE-CREATION > core/src/main/scala/kafka/server/KafkaApis.scala > ad6f05807c61c971e5e60d24bc0c87e989115961 > core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION > core/src/test/scala/unit/kafka/admin/AdminTest.scala > 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 > core/src/test/scala/unit/kafka/server/TopicCommandHelperTest.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/34766/diff/ > > > Testing > ------- > > > Thanks, > > Andrii Biletskyi > >
