> On March 29, 2015, 7:33 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/protocol/ApiVersion.java, > > line 28 > > <https://reviews.apache.org/r/28769/diff/21/?file=908349#file908349line28> > > > > Since this is for intra-broker communication, should we move this class > > to core?
Sure. At first I thought we'll need it when we move all requests/responses to o.a.k.common, but on second look, it doesn't seem like the requests/responses themselves need it. > On March 29, 2015, 7:33 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, lines 345-347 > > <https://reviews.apache.org/r/28769/diff/21/?file=908381#file908381line345> > > > > It seems that a 3-digit value like 0.8.2 is also valid? yes, and also 0.8.2.X or 0.8.2.Y. We treat them all as equivalent. Do you think its an issue? or just want to document this fact? > On March 29, 2015, 7:33 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/Utils.scala, line 614 > > <https://reviews.apache.org/r/28769/diff/21/?file=908392#file908392line614> > > > > Should we just return scala.collection.Map? Isn't it a bit confusing? A Map could be mutable, immutable, or "generic" (also immutable but a different type...) depending on what is imported. We pass the results of this around a bit between classes, sometimes relying on type-inference, often with different collections imported. I'd rather be stricter here, unless you see an issue with this? > On March 29, 2015, 7:33 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/cluster/BrokerTest.scala, lines 62-64 > > <https://reviews.apache.org/r/28769/diff/21/?file=908400#file908400line62> > > > > Is this necessary given the above tests on equal and hashCode? Probably not. Since we typically care most about the ability to find the object in a hashmap, I usually test for it explicitly. If you think it doesn't belong in Kafka, I can remove it. > On March 29, 2015, 7:33 p.m., Jun Rao wrote: > > system_test/run_all.sh, lines 5-6 > > <https://reviews.apache.org/r/28769/diff/21/?file=908434#file908434line5> > > > > Would it be better to just have testcase_to_run.json be > > testcase_to_run_all.json? Also, could we change README accordingly? Will update the readme. We can't just have testcase_to_run.json be testcase_to_run_all.json. system_test_runner executes testcase_to_run.json by default. We have several testsuites we may want to run testcase_to_run_all.json, testcase_to_run_all_replica.json (which I missed in this patch), and testcase_to_run_sanity.json. The shellscript backsup the existing test configuration, replaces with the tests we want to run and executes system_test_runner.sh (This is already done in run_sanity.sh) I agree that modifying system_test_runner to take the script as parameter will be cleaner, but this is beyond the scope of this patch. That is, I don't want to dive into the python spaggetti that will be replaced soon :) - Gwen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28769/#review78160 ----------------------------------------------------------- On March 27, 2015, 10:04 p.m., Gwen Shapira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28769/ > ----------------------------------------------------------- > > (Updated March 27, 2015, 10:04 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1809 > https://issues.apache.org/jira/browse/KAFKA-1809 > > > Repository: kafka > > > Description > ------- > > forgot rest of patch > > > merge with trunk > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > fa9daaef66ff7961e1c46cd0cd8fed18a53bccd8 > clients/src/main/java/org/apache/kafka/common/protocol/ApiVersion.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/utils/Utils.java > 920b51a6c3c99639fbc9dc065637eeee3c19fabd > clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java > c899813d55b9c4786adde3d840f040d6645d27c8 > config/server.properties 1614260b71a658b405bb24157c8f12b1f1031aa5 > core/src/main/scala/kafka/admin/AdminUtils.scala > b700110f2d7f1ede235af55d8e37e1b5592c6c7d > core/src/main/scala/kafka/admin/TopicCommand.scala > f400b71f8444fffd3fc1d8398a283682390eba4e > core/src/main/scala/kafka/api/ConsumerMetadataResponse.scala > 24aaf954dc42e2084454fa5fc9e8f388ea95c756 > 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.scala > 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 > 152fda5d1dcdf319399fdeeb8457006090ebe56c > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > b1cf0db8741e384e4e4119751058ea87b2589e57 > core/src/main/scala/kafka/controller/ControllerChannelManager.scala > c582191636f6188c25d62a67ff0315b56f163133 > core/src/main/scala/kafka/controller/KafkaController.scala > 09fc46d759b74bcdad2d2a610d9c5a93ff02423f > core/src/main/scala/kafka/javaapi/ConsumerMetadataResponse.scala > d281bb31a66fd749ecddfbe38479b6903f436831 > 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 > 76ce41aed6e04ac5ba88395c4d5008aca17f9a73 > 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 > 5d5cf5897cc37b3595f14bfe9d7cde43456bcc4b > core/src/main/scala/kafka/server/KafkaApis.scala > c33e848256be6159a148319bf74bf1590f27cc06 > core/src/main/scala/kafka/server/KafkaConfig.scala > 422451aec5ea0442eb2e4c1ae772885b813904a9 > core/src/main/scala/kafka/server/KafkaHealthcheck.scala > 7907987e43404487382de7f4cc294f0d01ac15a7 > core/src/main/scala/kafka/server/KafkaServer.scala > 4db3384545be8c237d6fc9646716ab67d5193ec5 > core/src/main/scala/kafka/server/MetadataCache.scala > 6aef6e4508ecadbbcc1e12bed2054547b7aa333e > core/src/main/scala/kafka/server/ReplicaFetcherManager.scala > 351dbbad3bdb709937943b336a5b0a9e0162a5e2 > core/src/main/scala/kafka/server/ReplicaFetcherThread.scala > 96faa7b4ed7c9ba8a3f6f9f114bd94e19b3a7ac0 > core/src/main/scala/kafka/server/ReplicaManager.scala > 44f0026e6676d8d764dd59dbcc6bb7bb727a3ba6 > core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala > d1e7c434e77859d746b8dc68dd5d5a3740425e79 > 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 > 7ae999ec619443d35a9cb8fbcd531fca0c51c8c0 > core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala > 7eb6d05b66b16f8b0ab64eb43dcc1a25168fa5be > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > 3df450784592b894008e7507b2737f9bb07f7bd2 > core/src/test/scala/other/kafka/TestOffsetManager.scala > a106379d96ab3cbdf54a9f162db44a6df177e733 > core/src/test/scala/unit/kafka/KafkaConfigTest.scala > 4d36b8b1173f60d43463c13c9d8c1275a84c8c28 > core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala > 8bc178505e00932a316c46ed1d904bd57b5b3f75 > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > 4cb803f3e86dbc97f1749b2db9788fe9d376d49a > core/src/test/scala/unit/kafka/cluster/BrokerTest.scala PRE-CREATION > core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala > 995397ba2e2dfc6fadd9d5c5efd90f2c4ac0d59c > core/src/test/scala/unit/kafka/integration/FetcherTest.scala > 3093e459935ecf8e5b34fca34a422674562a7034 > core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala > 062790f15d4ab6e0ee7a98bef12aaa2b22da3bfa > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala > a671af4a87d5c2fb42ff48c553bca7cae6538231 > core/src/test/scala/unit/kafka/log/LogTest.scala > 3c0599cc2ea12566fbec8233e3a73b31854b3b38 > core/src/test/scala/unit/kafka/network/SocketServerTest.scala > 0af23abf146d99e3d6cf31e5d6b95a9e63318ddb > core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala > be90c5bc7f1f5ba8a237d1c7176f27029727c918 > core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala > 812df5989940095c1a347066136ab55153bdad21 > core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala > 296e2b563041318d360db4037411879613a63968 > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > 191251d1340b5e5b2d649b37af3c6c1896d07e6e > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > 7f47e6f9a74314ed9e9f19d0c97931f3f2e49259 > core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala > f2528059dd86bb051d1ce9d72039cde3b78b8817 > core/src/test/scala/unit/kafka/server/LogOffsetTest.scala > 8c9f9e748e888da6035755c9f4241ac3aa0500c9 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 1682a77362123449de6bb1d54a55887409990a24 > > system_test/replication_testsuite/testcase_0001/testcase_0001_properties.json > 250ffe08ae4c2d35f7c79c883f47bbc27d5361b5 > > system_test/replication_testsuite/testcase_0002/testcase_0002_properties.json > 3f9e7d05db17e170aea4fee5a13ab5534dd5db2e > > system_test/replication_testsuite/testcase_0003/testcase_0003_properties.json > b10c6266a786acf0c4b1accb63f9727486d09fd9 > > system_test/replication_testsuite/testcase_0004/testcase_0004_properties.json > 22c08023f3d9e7678fdb990518bdb7931b64b6ac > > system_test/replication_testsuite/testcase_0005/testcase_0005_properties.json > 1317d8460f82c3ff09d5a13a1277d695ba71d6e7 > > system_test/replication_testsuite/testcase_0006/testcase_0006_properties.json > d31346506a19dd066519303a19a191f70599918d > > system_test/replication_testsuite/testcase_0007/testcase_0007_properties.json > bd00f130d7fc69dc43cf088e298abc43e51bf765 > > system_test/replication_testsuite/testcase_0008/testcase_0008_properties.json > ac5c13b09a84f975e345b635cf44a76935251244 > > system_test/replication_testsuite/testcase_0009/testcase_0009_properties.json > 6dadd7a288664ebdc7d023c54d9cdb5a54d0c2e7 > > system_test/replication_testsuite/testcase_0101/testcase_0101_properties.json > 614cb1cf01d3c76d8a97c2b52fb52bd83ec80c4b > > system_test/replication_testsuite/testcase_0102/testcase_0102_properties.json > fbf01772545f016c28448235a5859429a3a83780 > > system_test/replication_testsuite/testcase_0103/testcase_0103_properties.json > 317a6e3d3384009668e822e6c7e38b3fa34fcd6e > > system_test/replication_testsuite/testcase_0104/testcase_0104_properties.json > d1a790b34fa5b42fe9fce2dabf6833048f9905ea > > system_test/replication_testsuite/testcase_0105/testcase_0105_properties.json > 8f4f8bfac71463bbe52906ee7a127d8b0b5f48a3 > > system_test/replication_testsuite/testcase_0106/testcase_0106_properties.json > a307f855c944946724cf3a91a9c07cc7b57b71f6 > > system_test/replication_testsuite/testcase_0107/testcase_0107_properties.json > 827319eb6c6dbba0c8000b6e5e5a657a112deb21 > > system_test/replication_testsuite/testcase_0108/testcase_0108_properties.json > ae015cd0eb82b3d3f04e14c808b66dad702f7bf0 > > system_test/replication_testsuite/testcase_0109/testcase_0109_properties.json > 1951a1220956f931032e8fcd214784d76472054b > system_test/replication_testsuite/testcase_1/testcase_1_properties.json > 0c6d7a316cc6b51ac0755ca03558507db0706c31 > system_test/run_all.sh PRE-CREATION > system_test/run_all_replica.sh PRE-CREATION > system_test/utils/kafka_system_test_utils.py > 41d511cbc310fa87e0f2cd2f772e479e8e3ae4e2 > > Diff: https://reviews.apache.org/r/28769/diff/ > > > Testing > ------- > > > Thanks, > > Gwen Shapira > >
