Good idea :)

Fixed this, rebased the patch and uploaded a new diff.

Gwen

On Thu, Jan 29, 2015 at 9:27 AM, Don Bosco Durai <bo...@apache.org> wrote:
> +1
>
> I also feel, having security.* would be easy going forward.
>
> Thanks
>
> Bosco
>
>
> On 1/29/15, 6:08 AM, "Jeff Holoman" <jeff.holo...@gmail.com> wrote:
>
>>
>>
>>> On Jan. 23, 2015, 1:57 a.m., Jun Rao wrote:
>>> > core/src/main/scala/kafka/server/KafkaConfig.scala, lines 182-183
>>> >
>>><https://reviews.apache.org/r/28769/diff/12/?file=820431#file820431line18
>>>2>
>>> >
>>> >     Since this is also used for communication btw the controller and
>>>the brokers, perhaps it's better named as sth like
>>>"intra.broker.security.protocol"?
>>
>>Maybe it makese sense to prepend all security related configs with
>>"security, eg: "security.intra.broker.protocol",
>>"security.new.param.for.future.jira" With all of the upcoming changes it
>>would make security realted configs easy to spot.
>>
>>
>>- Jeff
>>
>>
>>-----------------------------------------------------------
>>This is an automatically generated e-mail. To reply, visit:
>>https://reviews.apache.org/r/28769/#review69281
>>-----------------------------------------------------------
>>
>>
>>On Jan. 28, 2015, 6:26 p.m., Gwen Shapira wrote:
>>>
>>> -----------------------------------------------------------
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/28769/
>>> -----------------------------------------------------------
>>>
>>> (Updated Jan. 28, 2015, 6:26 p.m.)
>>>
>>>
>>> Review request for kafka.
>>>
>>>
>>> Bugs: KAFKA-1809
>>>     https://issues.apache.org/jira/browse/KAFKA-1809
>>>
>>>
>>> Repository: kafka
>>>
>>>
>>> Description
>>> -------
>>>
>>> KAFKA-1890 Fix bug preventing Mirror Maker from successful rebalance;
>>>reviewed by Gwen Shapira and Neha Narkhede
>>>
>>>
>>> first commit of refactoring.
>>>
>>>
>>> changed topicmetadata to include brokerendpoints and fixed few unit
>>>tests
>>>
>>>
>>> fixing systest and support for binding to default address
>>>
>>>
>>> fixed system tests
>>>
>>>
>>> fix default address binding and ipv6 support
>>>
>>>
>>> fix some issues regarding endpoint parsing. Also, larger segments for
>>>systest make the validation much faster
>>>
>>>
>>> added link to security wiki in doc
>>>
>>>
>>> fixing unit test after rename of ProtocolType to SecurityProtocol
>>>
>>>
>>> Following Joe's advice, added security protocol enum on client side,
>>>and modified protocol to use ID instead of string.
>>>
>>>
>>> validate producer config against enum
>>>
>>>
>>> add a second protocol for testing and modify SocketServerTests to check
>>>on multi-ports
>>>
>>>
>>> Reverted the metadata request changes and removed the explicit security
>>>protocol argument. Instead the socketserver will determine the protocol
>>>based on the port and add this to the request
>>>
>>>
>>> bump version for UpdateMetadataRequest and added support for rolling
>>>upgrades with new config
>>>
>>>
>>> following tests - fixed LeaderAndISR protocol and ZK registration for
>>>backward compatibility
>>>
>>>
>>> cleaned up some changes that were actually not necessary. hopefully
>>>making this patch slightly easier to review
>>>
>>>
>>> undoing some changes that don't belong here
>>>
>>>
>>> bring back config lost in cleanup
>>>
>>>
>>> fixes neccessary for an all non-plaintext cluster to work
>>>
>>>
>>> minor modifications following comments by Jun
>>>
>>>
>>> added missing license
>>>
>>>
>>> formatting
>>>
>>>
>>> clean up imports
>>>
>>>
>>> cleaned up V2 to not include host+port field. Using use.new.protocol
>>>flag to decide which version to serialize
>>>
>>>
>>> change endpoints collection in Broker to Map[protocol, endpoint],
>>>mostly to be clear that we intend to have one endpoint per protocol
>>>
>>>
>>> validate that listeners and advertised listeners have unique ports and
>>>protocols
>>>
>>>
>>> support legacy configs
>>>
>>>
>>> some fixes following rebase
>>>
>>>
>>> Reverted to backward compatible zk registration, changed
>>>use.new.protocol to support multiple versions and few minor fixes
>>>
>>>
>>> Diffs
>>> -----
>>>
>>>
>>>clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.ja
>>>va 8b3e565edd1ae04d8d34bd9f1a41e9fa8c880a75
>>>
>>>clients/src/main/java/org/apache/kafka/common/protocol/ApiVersion.java
>>>PRE-CREATION
>>>
>>>clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.j
>>>ava PRE-CREATION
>>>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java
>>>527dd0f9c47fce7310b7a37a9b95bf87f1b9c292
>>>   clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java
>>>a39fab532f73148316a56c0f8e9197f38ea66f79
>>>   config/server.properties 1614260b71a658b405bb24157c8f12b1f1031aa5
>>>   core/src/main/scala/kafka/admin/AdminUtils.scala
>>>28b12c7b89a56c113b665fbde1b95f873f8624a3
>>>   core/src/main/scala/kafka/admin/TopicCommand.scala
>>>285c0333ff43543d3e46444c1cd9374bb883bb59
>>>   core/src/main/scala/kafka/api/ConsumerMetadataResponse.scala
>>>84f60178f6ebae735c8aa3e79ed93fe21ac4aea7
>>>   core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala
>>>4ff7e8f8cc695551dd5d2fe65c74f6b6c571e340
>>>   core/src/main/scala/kafka/api/TopicMetadata.scala
>>>0190076df0adf906ecd332284f222ff974b315fc
>>>   core/src/main/scala/kafka/api/TopicMetadataResponse.scala
>>>92ac4e687be22e4800199c0666bfac5e0059e5bb
>>>   core/src/main/scala/kafka/api/UpdateMetadataRequest.scala
>>>530982e36b17934b8cc5fb668075a5342e142c59
>>>   core/src/main/scala/kafka/client/ClientUtils.scala
>>>ebba87f0566684c796c26cb76c64b4640a5ccfde
>>>   core/src/main/scala/kafka/cluster/Broker.scala
>>>0060add008bb3bc4b0092f2173c469fce0120be6
>>>   core/src/main/scala/kafka/cluster/BrokerEndPoint.scala PRE-CREATION
>>>   core/src/main/scala/kafka/cluster/EndPoint.scala PRE-CREATION
>>>   core/src/main/scala/kafka/cluster/SecurityProtocol.scala PRE-CREATION
>>>
>>>core/src/main/scala/kafka/common/BrokerEndPointNotAvailableException.scal
>>>a PRE-CREATION
>>>   core/src/main/scala/kafka/consumer/ConsumerConfig.scala
>>>9ebbee6c16dc83767297c729d2d74ebbd063a993
>>>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala
>>>b9e2bea7b442a19bcebd1b350d39541a8c9dd068
>>>   core/src/main/scala/kafka/consumer/ConsumerFetcherThread.scala
>>>ee6139c901082358382c5ef892281386bf6fc91b
>>>   core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala
>>>5487259751ebe19f137948249aa1fd2637d2deb4
>>>   core/src/main/scala/kafka/controller/ControllerChannelManager.scala
>>>eb492f00449744bc8d63f55b393e2a1659d38454
>>>   core/src/main/scala/kafka/controller/KafkaController.scala
>>>66df6d2fbdbdd556da6bea0df84f93e0472c8fbf
>>>   core/src/main/scala/kafka/javaapi/ConsumerMetadataResponse.scala
>>>1b28861cdf7dfb30fc696b54f8f8f05f730f4ece
>>>   core/src/main/scala/kafka/javaapi/TopicMetadata.scala
>>>f384e04678df10a5b46a439f475c63371bf8e32b
>>>   core/src/main/scala/kafka/network/RequestChannel.scala
>>>7b1db3dbbb2c0676f166890f566c14aa248467ab
>>>   core/src/main/scala/kafka/network/SocketServer.scala
>>>39b1651b680b2995cedfde95d74c086d9c6219ef
>>>   core/src/main/scala/kafka/producer/ProducerPool.scala
>>>43df70bb461dd3e385e6b20396adef3c4016a3fc
>>>   core/src/main/scala/kafka/server/AbstractFetcherManager.scala
>>>20c00cb8cc2351950edbc8cb1752905a0c26e79f
>>>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala
>>>8c281d4668f92eff95a4a5df3c03c4b5b20e7095
>>>   core/src/main/scala/kafka/server/KafkaApis.scala
>>>ec8d9f7ba44741db40875458bd524c4062ad6a26
>>>   core/src/main/scala/kafka/server/KafkaConfig.scala
>>>6d74983472249eac808d361344c58cc2858ec971
>>>   core/src/main/scala/kafka/server/KafkaHealthcheck.scala
>>>4acdd70fe9c1ee78d6510741006c2ece65450671
>>>   core/src/main/scala/kafka/server/KafkaServer.scala
>>>89200da30a04943f0b9befe84ab17e62b747c8c4
>>>   core/src/main/scala/kafka/server/MetadataCache.scala
>>>bf81a1ab88c14be8697b441eedbeb28fa0112643
>>>   core/src/main/scala/kafka/server/ReplicaFetcherManager.scala
>>>351dbbad3bdb709937943b336a5b0a9e0162a5e2
>>>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala
>>>6879e730282185bda3d6bc3659cb15af0672cecf
>>>   core/src/main/scala/kafka/server/ReplicaManager.scala
>>>e58fbb922e93b0c31dff04f187fcadb4ece986d7
>>>   core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala
>>>d1e7c434e77859d746b8dc68dd5d5a3740425e79
>>>   core/src/main/scala/kafka/tools/MirrorMaker.scala
>>>5cbc8103e33a0a234d158c048e5314e841da6249
>>>   core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala
>>>ba6ddd7a909df79a0f7d45e8b4a2af94ea0fceb6
>>>   core/src/main/scala/kafka/tools/SimpleConsumerShell.scala
>>>b4f903b6c7c3bb725cac7c05eb1f885906413c4d
>>>   core/src/main/scala/kafka/tools/UpdateOffsetsInZK.scala
>>>111c9a8b94ce45d95551482e9fd3f8c1cccbf548
>>>   core/src/main/scala/kafka/utils/Utils.scala
>>>738c1af9ef5de16fdf5130daab69757a14c48b5c
>>>   core/src/main/scala/kafka/utils/ZkUtils.scala
>>>c14bd455b6642f5e6eb254670bef9f57ae41d6cb
>>>
>>>core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.sca
>>>la 90c0b7a19c7af8e5416e4bdba62b9824f1abd5ab
>>>   core/src/test/scala/integration/kafka/api/ProducerSendTest.scala
>>>b15237b76def3b234924280fa3fdb25dbb0cc0dc
>>>   core/src/test/scala/other/kafka/TestOffsetManager.scala
>>>41f334d48897b3027ed54c58bbf4811487d3b191
>>>   core/src/test/scala/unit/kafka/KafkaConfigTest.scala
>>>4d36b8b1173f60d43463c13c9d8c1275a84c8c28
>>>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala
>>>1bf2667f47853585bc33ffb3e28256ec5f24ae84
>>>
>>>core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala
>>> cd16ced5465d098be7a60498326b2a98c248f343
>>>   core/src/test/scala/unit/kafka/cluster/BrokerTest.scala PRE-CREATION
>>>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala
>>>c0355cc0135c6af2e346b4715659353a31723b86
>>>   core/src/test/scala/unit/kafka/integration/FetcherTest.scala
>>>25845abbcad2e79f56f729e59239b738d3ddbc9d
>>>   core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala
>>>35dc071b1056e775326981573c9618d8046e601d
>>>   core/src/test/scala/unit/kafka/log/LogTest.scala
>>>c2dd8eb69da8c0982a0dd20231c6f8bd58eb623e
>>>   core/src/test/scala/unit/kafka/network/SocketServerTest.scala
>>>78b431f9c88cca1bc5e430ffd41083d0e92b7e75
>>>   core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala
>>>1db6ac329f7b54e600802c8a623f80d159d4e69b
>>>   core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala
>>>d60d8e0f49443f4dc8bc2cad6e2f951eda28f5cb
>>>   core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala
>>>f0c4a56b61b4f081cf4bee799c6e9c523ff45e19
>>>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
>>>82dce80d553957d8b5776a9e140c346d4e07f766
>>>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala
>>>c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e
>>>   core/src/test/scala/unit/kafka/server/LogOffsetTest.scala
>>>c06ee756bf0fe07e5d3c92823a476c960b37afd6
>>>   core/src/test/scala/unit/kafka/utils/TestUtils.scala
>>>ac15d34425795d5be20c51b01fa1108bdcd66583
>>>
>>>system_test/replication_testsuite/testcase_1/testcase_1_properties.json
>>>0c6d7a316cc6b51ac0755ca03558507db0706c31
>>>   system_test/utils/kafka_system_test_utils.py
>>>41d511cbc310fa87e0f2cd2f772e479e8e3ae4e2
>>>
>>> Diff: https://reviews.apache.org/r/28769/diff/
>>>
>>>
>>> Testing
>>> -------
>>>
>>>
>>> Thanks,
>>>
>>> Gwen Shapira
>>>
>>>
>>
>
>

Reply via email to