brandboat commented on code in PR #20385:
URL: https://github.com/apache/kafka/pull/20385#discussion_r2303311048
##########
tools/src/main/java/org/apache/kafka/tools/ShareConsumerPerformance.java:
##########
@@ -366,7 +380,20 @@ public ShareConsumerPerfOptions(String[] args) {
}
if (options != null) {
CommandLineUtils.maybePrintHelpOrVersion(this, "This tool is
used to verify the share consumer performance.");
- CommandLineUtils.checkRequiredArgs(parser, options, topicOpt,
numMessagesOpt);
+ CommandLineUtils.checkRequiredArgs(parser, options, topicOpt);
+
+ CommandLineUtils.checkOneOfArgs(parser, options,
numMessagesOpt, numRecordsOpt);
+ CommandLineUtils.checkInvalidArgs(parser, options,
consumerConfigOpt, commandConfigOpt);
+
+ if (options.has(numMessagesOpt)) {
+ System.out.println("Warning: --messages is deprecated. Use
--num-records instead.");
+ }
+
+ if (options.has(consumerConfigOpt)) {
+ System.out.println("Warning: --consumer.config is
deprecated. Use --command-config instead.");
+ }
+
+
Review Comment:
nit: could you please the extra blank lines?
##########
tools/src/test/java/org/apache/kafka/tools/ShareConsumerPerformanceTest.java:
##########
@@ -62,23 +62,62 @@ public void testConfigBootStrapServer() {
String[] args = new String[]{
"--bootstrap-server", "localhost:9092",
"--topic", "test",
- "--messages", "10",
+ "--num-records", "10",
"--print-metrics"
};
ShareConsumerPerformance.ShareConsumerPerfOptions config = new
ShareConsumerPerformance.ShareConsumerPerfOptions(args);
assertEquals("localhost:9092", config.brokerHostsAndPorts());
assertTrue(config.topic().contains("test"));
- assertEquals(10, config.numMessages());
+ assertEquals(10, config.numRecords());
}
@Test
- public void testConfigWithUnrecognizedOption() {
+ public void testNumOfRecordsNotPresent() {
+ String[] args = new String[]{
+ "--bootstrap-server", "localhost:9092",
+ "--topic", "test"
+ };
+
+ String err = ToolsTestUtils.captureStandardErr(() ->
+ new ShareConsumerPerformance.ShareConsumerPerfOptions(args));
+ assertTrue(err.contains("Exactly one of the following arguments is
required:"));
+ }
+
+ @Test
+ public void testNumOfRecordsDeprecated() {
Review Comment:
```suggestion
public void testMessagesDeprecated() {
```
##########
tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java:
##########
@@ -136,7 +174,9 @@ public void testConfigWithoutTopicAndInclude() {
@Test
public void testClientIdOverride() throws IOException {
- File tempFile =
Files.createFile(tempDir.resolve("test_consumer_config.conf")).toFile();
+ Path configPath = tempDir.resolve("test_consumer_config.conf");
+ Files.deleteIfExists(configPath);
Review Comment:
This is not required, tempDir will be cleanup automatically after junit test
finished.
Same thing in other places.
##########
tools/src/test/java/org/apache/kafka/tools/ShareConsumerPerformanceTest.java:
##########
@@ -98,21 +139,62 @@ public void testClientIdOverride() throws IOException {
String[] args = new String[]{
"--bootstrap-server", "localhost:9092",
"--topic", "test",
- "--messages", "10",
- "--consumer.config", tempFile.getAbsolutePath()
+ "--num-records", "10",
+ "--command-config", tempFile.getAbsolutePath()
};
ShareConsumerPerformance.ShareConsumerPerfOptions config = new
ShareConsumerPerformance.ShareConsumerPerfOptions(args);
assertEquals("share-consumer-1",
config.props().getProperty(ConsumerConfig.CLIENT_ID_CONFIG));
+ Files.deleteIfExists(configPath);
+ }
+
+ @Test
+ public void testCommandConfigDeprecated() throws IOException {
Review Comment:
```suggestion
public void testConsumerConfigDeprecated() throws IOException {
```
##########
tools/src/test/java/org/apache/kafka/tools/ShareConsumerPerformanceTest.java:
##########
@@ -89,7 +128,9 @@ public void testConfigWithUnrecognizedOption() {
@Test
public void testClientIdOverride() throws IOException {
- File tempFile =
Files.createFile(tempDir.resolve("test_share_consumer_config.conf")).toFile();
+ Path configPath = tempDir.resolve("test_share_consumer_config.conf");
+ Files.deleteIfExists(configPath);
Review Comment:
We can remove this line. same thing in other places.
##########
tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java:
##########
@@ -145,21 +185,61 @@ public void testClientIdOverride() throws IOException {
String[] args = new String[]{
"--bootstrap-server", "localhost:9092",
"--topic", "test",
- "--messages", "10",
+ "--num-records", "10",
+ "--command-config", tempFile.getAbsolutePath()
+ };
+
+ ConsumerPerformance.ConsumerPerfOptions config = new
ConsumerPerformance.ConsumerPerfOptions(args);
+
+ assertEquals("consumer-1",
config.props().getProperty(ConsumerConfig.CLIENT_ID_CONFIG));
+ Files.deleteIfExists(configPath);
+ }
+
+ @Test
+ public void testCommandConfigDeprecated() throws IOException {
Review Comment:
```suggestion
public void testConsumerConfigDeprecated() throws IOException {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
