You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Joe Stein <cr...@gmail.com> on 2015/01/06 09:19:48 UTC
Re: Review Request 29301: Patch for KAFKA-1694
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29301/#review66801
-----------------------------------------------------------
build.gradle
<https://reviews.apache.org/r/29301/#comment110428>
We need to have upload archive here
build.gradle
<https://reviews.apache.org/r/29301/#comment110431>
If we can do this without an upgrade that would be great if we are in fact just requiring 1 function.
build.gradle
<https://reviews.apache.org/r/29301/#comment110432>
???
clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java
<https://reviews.apache.org/r/29301/#comment110437>
not sure about this name, you are making an Option[] for the protocol value and that not make sense if you look at the use of it and not how it works.
clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java
<https://reviews.apache.org/r/29301/#comment110439>
why wouldn't the null size == 0?
core/src/main/scala/kafka/common/ErrorMapping.scala
<https://reviews.apache.org/r/29301/#comment110440>
This is for trying to send an admin request to a broker that is not the controller? I think it should be name for that more specifically and clearly/
- Joe Stein
On Dec. 24, 2014, 7:22 p.m., Andrii Biletskyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29301/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2014, 7:22 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1694
> https://issues.apache.org/jira/browse/KAFKA-1694
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1694 - introduced new type for Wire protocol, ported ClusterMetadataResponse to it
>
>
> KAFKA-1694 - Split Admin RQ/RP to separate messages
>
>
> KAFKA-1694 - Admin commands can be handled only by controller; DeleteTopicCommand NPE fix
>
>
> Diffs
> -----
>
> bin/kafka.sh PRE-CREATION
> bin/windows/kafka.bat PRE-CREATION
> build.gradle 18f86e4c8a10618d50ac78572d119c6e100ed85b
> clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c
> clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a
> clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc
> clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataResponse.java PRE-CREATION
> 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/main/java/org/apache/kafka/common/requests/admin/DescribeTopicOutput.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsOutput.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/TopicConfigDetails.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionsDetails.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java df37fc6d8f0db0b8192a948426af603be3444da4
> core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca
> core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43
> core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/CreateTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/CreateTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DeleteTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DeleteTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DescribeTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DescribeTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ListTopicsRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ListTopicsResponse.scala PRE-CREATION
> core/src/main/scala/kafka/common/AdminRequestFailedException.scala PRE-CREATION
> core/src/main/scala/kafka/common/ErrorMapping.scala eedc2f5f21dd8755fba891998456351622e17047
> core/src/main/scala/kafka/common/InvalidRequestTargetException.scala PRE-CREATION
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala eb492f00449744bc8d63f55b393e2a1659d38454
> core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf
> core/src/main/scala/kafka/server/KafkaApis.scala 2a1c0326b6e6966d8b8254bd6a1cb83ad98a3b80
> core/src/main/scala/kafka/server/MetadataCache.scala bf81a1ab88c14be8697b441eedbeb28fa0112643
> core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION
> core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala cd16ced5465d098be7a60498326b2a98c248f343
> settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45
> tools/src/main/java/org/apache/kafka/cli/BaseCommandOpts.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/Boot.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/RequestDispatcher.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/Shell.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/AlterTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ClearScreenCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/Command.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/CreateTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/DeleteTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/DescribeTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ExitCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ListTopicsCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/PrintHelpCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/TopicSwitchCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/util/StringUtils.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29301/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrii Biletskyi
>
>
Re: Review Request 29301: Patch for KAFKA-1694
Posted by Andrii Biletskyi <an...@stealth.ly>.
> On Jan. 6, 2015, 8:19 a.m., Joe Stein wrote:
> > build.gradle, line 211
> > <https://reviews.apache.org/r/29301/diff/2/?file=800278#file800278line211>
> >
> > If we can do this without an upgrade that would be great if we are in fact just requiring 1 function.
This feature allows us to call CLI commands both from interactive shell (kafka>) and right from ./kafka.sh (e.g. ./kafka.sh--create-topic).
The difference is that for ./kafka.sh we always have to supply one additional option --controller which is not recognized during command execution from interactive Shell.
As a workaround I can process --controller separately and then simply "cut off" it from command line args array but this looks a bit lame.
> On Jan. 6, 2015, 8:19 a.m., Joe Stein wrote:
> > clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java, line 21
> > <https://reviews.apache.org/r/29301/diff/2/?file=800281#file800281line21>
> >
> > not sure about this name, you are making an Option[] for the protocol value and that not make sense if you look at the use of it and not how it works.
As discussed for now better naming option wasn't found:). I'm happy to change if someone comes up with a good one.
I also added some comments to the class to make it more explanatory.
> On Jan. 6, 2015, 8:19 a.m., Joe Stein wrote:
> > clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java, line 51
> > <https://reviews.apache.org/r/29301/diff/2/?file=800281#file800281line51>
> >
> > why wouldn't the null size == 0?
There is always an additional byte showing whether value is present.
- Andrii
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29301/#review66801
-----------------------------------------------------------
On Jan. 12, 2015, 4:55 p.m., Andrii Biletskyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29301/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2015, 4:55 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1694
> https://issues.apache.org/jira/browse/KAFKA-1694
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1694 - introduced new type for Wire protocol, ported ClusterMetadataResponse to it
>
>
> KAFKA-1694 - Split Admin RQ/RP to separate messages
>
>
> KAFKA-1694 - Admin commands can be handled only by controller; DeleteTopicCommand NPE fix
>
>
> KAFKA-1776 - Ported ConsumerGroupOffsetChecker
>
>
> KAFKA-1776 - Ported PreferredReplicaElectionTool and ReassignPartitionsTool to CLI
>
>
> KAFKA-1694 - kafka-tools is uploaded on uploadAllArchives
>
>
> KAFKA-1694 - ReviewBoard 29301 code review fixes
>
>
> Diffs
> -----
>
> bin/kafka.sh PRE-CREATION
> bin/windows/kafka.bat PRE-CREATION
> build.gradle ba52288031e2abc70e35e9297a4423dd5025950b
> clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c
> clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a
> clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc
> clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataResponse.java PRE-CREATION
> 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/ConsumerGroupOffsetsRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ConsumerGroupOffsetsResponse.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/main/java/org/apache/kafka/common/requests/admin/DescribeTopicOutput.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsOutput.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/PreferredReplicaLeaderElectionRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/PreferredReplicaLeaderElectionResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ReassignPartitionsRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/ReassignPartitionsResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/TopicConfigDetails.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionsDetails.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionResponse.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyReassignPartitionsRequest.java PRE-CREATION
> clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyReassignPartitionsResponse.java PRE-CREATION
> clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java df37fc6d8f0db0b8192a948426af603be3444da4
> core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca
> core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43
> core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ConsumerGroupOffsetsRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ConsumerGroupOffsetsResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/CreateTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/CreateTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DeleteTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DeleteTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DescribeTopicRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/DescribeTopicResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ListTopicsRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ListTopicsResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/PreferredReplicaLeaderElectionRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/PreferredReplicaLeaderElectionResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ReassignPartitionsRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/ReassignPartitionsResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/VerifyPreferredReplicaLeaderElectionRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/VerifyPreferredReplicaLeaderElectionResponse.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/VerifyReassignPartitionsRequest.scala PRE-CREATION
> core/src/main/scala/kafka/api/admin/VerifyReassignPartitionsResponse.scala PRE-CREATION
> core/src/main/scala/kafka/common/AdminRequestFailedException.scala PRE-CREATION
> core/src/main/scala/kafka/common/ErrorMapping.scala eedc2f5f21dd8755fba891998456351622e17047
> core/src/main/scala/kafka/common/InvalidRequestTargetException.scala PRE-CREATION
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala eb492f00449744bc8d63f55b393e2a1659d38454
> core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf
> core/src/main/scala/kafka/server/KafkaApis.scala c011a1b79bd6c4e832fe7d097daacb0d647d1cd4
> core/src/main/scala/kafka/server/MetadataCache.scala bf81a1ab88c14be8697b441eedbeb28fa0112643
> core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION
> core/src/main/scala/kafka/tools/ConsumerOffsetCheckerHelper.scala PRE-CREATION
> core/src/main/scala/kafka/tools/PreferredReplicaLeaderElectionHelper.scala PRE-CREATION
> core/src/main/scala/kafka/tools/ReassignPartitionsHelper.scala PRE-CREATION
> core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala cd16ced5465d098be7a60498326b2a98c248f343
> settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45
> tools/src/main/java/org/apache/kafka/cli/BaseCommandOpts.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/Boot.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/RequestDispatcher.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/Shell.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/AlterTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ClearScreenCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/Command.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/CreateTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/DeleteTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/DescribeTopicCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ExitCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ListTopicsCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/PreferredReplicaLeaderElectionCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/PrintConsumerGroupOffsetsCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/PrintHelpCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/ReassignPartitionsCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/command/TopicSwitchCommand.java PRE-CREATION
> tools/src/main/java/org/apache/kafka/cli/util/StringUtils.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29301/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrii Biletskyi
>
>