You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/12/31 06:44:02 UTC
[GitHub] [kafka] itantiger opened a new pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
itantiger opened a new pull request #9790:
URL: https://github.com/apache/kafka/pull/9790
Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala"
linked see https://issues.apache.org/jira/browse/KAFKA-10043
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### Committer Checklist (excluded from commit message)
- [x] Verify design and implementation
- [x] Verify test coverage and CI build status
- [x] Verify documentation (including upgrade notes)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751594582
> But i did not found unit tests for this class kafka.tools.ConsumerPerformance.scala
It would be great if you could be the first person to write the tests for ```ConsumerPerformance``` :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger removed a comment on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger removed a comment on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752872756
> >trunk? pardon me that I can't catch your point.
> That's my problem. hah
>> yep, the default value CAN overwrite the configs from either command-line or config file.
> Let me optimize it again and commit. Do you think it's necessary?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751609344
> what should I do next :)
please take a look at the report and fix the error. https://github.com/apache/kafka/pull/9790/checks?check_run_id=1616222522
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger removed a comment on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger removed a comment on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752869488
> I guess you are talking about "title". yes, the title need to be revised.
In addition, I think the trunk also needs to be modified.
Some config params would be overwritten by default value, WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752864933
> > and then call `props.putIfAbsent` to set args from command-line only if the prop is nonexistent in config file. Hence, the priority of config file is higher than command-line, right?
> > You're right. I've ignored it. It should be modified here. :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#discussion_r549578153
##########
File path: core/src/test/scala/unit/kafka/tools/ConsumerPerformanceTest.scala
##########
@@ -96,6 +97,28 @@ class ConsumerPerformanceTest {
assertEquals("test", config.topic)
assertEquals(10, config.numMessages)
}
+
+ @Test
+ def testConfigWithRecognizedOptionOverride(): Unit = {
+ val propsFile = TestUtils.tempFile()
+ val propsStream = Files.newOutputStream(propsFile.toPath)
+ propsStream.write("group.id=test_group_id\n".getBytes())
+ propsStream.write("client.id=test_client_id".getBytes())
+ propsStream.close()
Review comment:
Could you test ```--topic``` to make sure it is not overrided by command-line arguments.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751664539
@itantiger Thanks for your patch. Could you revise the title?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752870058
> I guess you are talking about "title". yes, the title need to be revised.
In addition, I think the trunk also needs to be modified.
Some config params would be overwritten by default value, WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752866046
> Do you think it needs to be revised?
I guess you are talking about "title". yes, the title need to be revised.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752856869
@itantiger The priority of loading props is a bit chaos in this case. It seems to me the property of loading properties is shown below.
1. command-lind
1. config file (consumer.config)
1. default value
The current priority is shown below.
1. command-lind
1. default value
1. config file (consumer.config)
The priority from this PR is shown below.
1. config file (consumer.config)
1. command-lind
1. default value
Of course, current priority has issue as properties from config file always get overwritten by default value. We need to fix it. However, I'd like to check the priority again. WDYT? Should config file has higher priority than command-lind?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#discussion_r549591650
##########
File path: core/src/test/scala/unit/kafka/tools/ConsumerPerformanceTest.scala
##########
@@ -96,6 +97,28 @@ class ConsumerPerformanceTest {
assertEquals("test", config.topic)
assertEquals(10, config.numMessages)
}
+
+ @Test
+ def testConfigWithRecognizedOptionOverride(): Unit = {
+ val propsFile = TestUtils.tempFile()
+ val propsStream = Files.newOutputStream(propsFile.toPath)
+ propsStream.write("group.id=test_group_id\n".getBytes())
+ propsStream.write("client.id=test_client_id".getBytes())
+ propsStream.close()
Review comment:
Okey, I finished the local test and submitted the code :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751665759
> @itantiger Thanks for your patch. Could you revise the title?
Also thank you very much for your guidance :) .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752872756
> >trunk? pardon me that I can't catch your point.
> That's my problem. hah
>> yep, the default value CAN overwrite the configs from either command-line or config file.
> Let me optimize it again and commit. Do you think it's necessary?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751664125
> > what should I do next :)
>
> please take a look at the report and fix the error. https://github.com/apache/kafka/pull/9790/checks?check_run_id=1616222522
I fixed the error, but there were many errors in the second build. It seemed that the errors had nothing to do with what I changed. Please help me to have a look :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751602370
> > But i did not found unit tests for this class kafka.tools.ConsumerPerformance.scala
>
> It would be great if you could be the first person to write the tests for `ConsumerPerformance` :)
I had committed the unit test in `ConsumerPerformanceTest`, What should I do to relate to that?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751608658
> > > But i did not found unit tests for this class kafka.tools.ConsumerPerformance.scala
> >
> >
> > It would be great if you could be the first person to write the tests for `ConsumerPerformance` :)
>
> I had committed the unit test in `ConsumerPerformanceTest`, What should I do to relate to that?
I found that it was automatically linked，but all checks had failed :( .
what should I do next :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752870565
> I think the trunk also needs to be modified.
trunk? pardon me that I can't catch your point.
> Some config params would be overwritten by default value, WDYT?
yep, the default value CAN overwrite the configs from either command-line or config file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752873484
> I'd like to address the following priority rule in this PR :)
>
> 1. command-lind
> 2. config file (consumer.config)
> 3. default value
Of course, I also think this is more reasonable
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751591629
> Could you add test for this issue?
Do you mean to add unit tests？But i did not found unit tests for this class `kafka.tools.ConsumerPerformance.scala`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#discussion_r549583198
##########
File path: core/src/test/scala/unit/kafka/tools/ConsumerPerformanceTest.scala
##########
@@ -96,6 +97,28 @@ class ConsumerPerformanceTest {
assertEquals("test", config.topic)
assertEquals(10, config.numMessages)
}
+
+ @Test
+ def testConfigWithRecognizedOptionOverride(): Unit = {
+ val propsFile = TestUtils.tempFile()
+ val propsStream = Files.newOutputStream(propsFile.toPath)
+ propsStream.write("group.id=test_group_id\n".getBytes())
+ propsStream.write("client.id=test_client_id".getBytes())
+ propsStream.close()
Review comment:
`--topic` is a `REQUIRED` argument in command-line, I thank Its value is determined by the command line.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752869488
> I guess you are talking about "title". yes, the title need to be revised.
In addition, I think the trunk also needs to be modified.
Some config params would be overwritten by default value, WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#discussion_r549589413
##########
File path: core/src/test/scala/unit/kafka/tools/ConsumerPerformanceTest.scala
##########
@@ -96,6 +97,28 @@ class ConsumerPerformanceTest {
assertEquals("test", config.topic)
assertEquals(10, config.numMessages)
}
+
+ @Test
+ def testConfigWithRecognizedOptionOverride(): Unit = {
+ val propsFile = TestUtils.tempFile()
+ val propsStream = Files.newOutputStream(propsFile.toPath)
+ propsStream.write("group.id=test_group_id\n".getBytes())
+ propsStream.write("client.id=test_client_id".getBytes())
+ propsStream.close()
Review comment:
Or could we add ```group.id``` to command-line arguments to make sure it (from file) is NOT override by command-line args?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger closed pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger closed pull request #9790:
URL: https://github.com/apache/kafka/pull/9790
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751479714
> Could you update the code (according to the reviews) and then push the changes to be a new commit. Also, please rebase your code (by trunk).
@chia7712 Can you take a look at this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#discussion_r550151238
##########
File path: core/src/test/scala/unit/kafka/tools/ConsumerPerformanceTest.scala
##########
@@ -96,6 +97,28 @@ class ConsumerPerformanceTest {
assertEquals("test", config.topic)
assertEquals(10, config.numMessages)
}
+
+ @Test
+ def testConfigWithRecognizedOptionOverride(): Unit = {
+ val propsFile = TestUtils.tempFile()
+ val propsStream = Files.newOutputStream(propsFile.toPath)
+ propsStream.write("group.id=test_group_id\n".getBytes())
+ propsStream.write("client.id=test_client_id".getBytes())
+ propsStream.close()
Review comment:
@chia7712 Please take a look :) Is there anything else that needs to be optimized？
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752863384
> and then call `props.putIfAbsent` to set args from command-line only if the prop is nonexistent in config file. Hence, the priority of config file is higher than command-line, right?
You're right. I've ignored it. It should be modified here. :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751673393
> @itantiger Thanks for your patch. Could you revise the title?
Finished.
When will this patch merge to trunk?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752872927
>>trunk? pardon me that I can't catch your point.
>
>That's my problem. hah
>
>>yep, the default value CAN overwrite the configs from either command-line or config file.
>
>Let me optimize it again and commit. Do you think it's necessary?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752865674
Current priority, some config params would be overwritten by default value, Do you think it needs to be revised?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752862049
> I think this priority is more better, and this PR is to do this.
We load the props from config file first. https://github.com/apache/kafka/blob/afa5423356d3d2a2135a51200573b45d097f6d60/core/src/main/scala/kafka/tools/ConsumerPerformance.scala#L275 and then call ```props.putIfAbsent``` to set args from command-line only if the prop is nonexistent in config file. Hence, the priority of config file is higher than command-line, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752873182
> Let me optimize it again and commit. Do you think it's necessary?
I'd like to address the following priority rule in this PR :)
1. command-lind
2. config file (consumer.config)
3. default value
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-752860409
> 1. command-lind
> 2. config file (consumer.config)
> 3. default value
I think this priority is more better, and this PR is to do this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9790: Some parameters will be overwritten which was configured in consumer.config where running "ConsumerPerformance.scala". Linked https://issues.apache.org/jira/browse/KAFKA-10043
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#issuecomment-751577802
Could you add test for this issue?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #9790: Some parameters will be overwritten which was configured in consumer.config.
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #9790:
URL: https://github.com/apache/kafka/pull/9790#discussion_r549583876
##########
File path: core/src/test/scala/unit/kafka/tools/ConsumerPerformanceTest.scala
##########
@@ -96,6 +97,28 @@ class ConsumerPerformanceTest {
assertEquals("test", config.topic)
assertEquals(10, config.numMessages)
}
+
+ @Test
+ def testConfigWithRecognizedOptionOverride(): Unit = {
+ val propsFile = TestUtils.tempFile()
+ val propsStream = Files.newOutputStream(propsFile.toPath)
+ propsStream.write("group.id=test_group_id\n".getBytes())
+ propsStream.write("client.id=test_client_id".getBytes())
+ propsStream.close()
Review comment:
So, if config file has the param `topic=test`, It will not take effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org