You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/09/22 15:49:40 UTC
[GitHub] [camel-quarkus] zbendhiba opened a new pull request #3124: Improve Kafka test coverage
zbendhiba opened a new pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124
Fixes #2627
<!-- Uncomment and fill this section if your PR is not trivial
[ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
[ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
[ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
[ ] Phrases like Fix #<issueNumber> or Fixes #<issueNumber> will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
[ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
[ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html
-->
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714827343
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
Thinking aloud:
We would have a kind of counter in the process() statement
The process statement would do something like that setBody(counter)
Send "content" to kafka, assert seda as seen counter = 1 and counter 2 (so delivered uncommitted and then delivered committed).
That way, we wouldn't need the last startup ? Would it cover everything needed thought ?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
zbendhiba commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r716794878
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
@aldettinger yes ! this is the test https://github.com/apache/camel/blob/9825f640a104cd506487109e8d351ef84dc25118/components/camel-kafka/src/test/java/org/apache/camel/component/kafka/integration/KafkaConsumerManualCommitIT.java#L84
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714786016
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
Ok, I think I got it. My understanding is that the trick to start/stop foo/bar routes is just to have the message committed or not.
So, I wonder if it would be clearer with a single route that would call `manual.commitSync();` each 2 messages only ?
So the first message would be committed, the second not and so on....
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r716820492
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
Ok, then I would say let's do this also.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
zbendhiba commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714841879
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
Yeah it could work. Let me try this.
I'm not either fan of starting and stopping Routes.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga merged pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
ppalaga merged pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga commented on pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#issuecomment-928254350
Ignoring the failing docs checks
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
zbendhiba commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714841879
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
Yeah it could work. Let me try this.
I'm not either a fan of starting and stopping Routes.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga commented on pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#issuecomment-928254350
Ignoring the failing docs checks
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
zbendhiba commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r716708823
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
I pushed a fix, to have only one single Route.
I still need to stop and restart this one to redeliver the message
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
aldettinger commented on pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#issuecomment-928000714
Well done @zbendhiba
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r716772935
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
I had the idea that it would work like jdbc, amq commits. But maybe in the context of kakfa it's something else. Maybe it's just an asynchronous commit that has the time to be executed with the %2 testing method. If any, does camel-kafka tests also stop/start routes ?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga merged pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
ppalaga merged pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] aldettinger commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
aldettinger commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714738718
##########
File path: integration-tests/kafka/src/main/java/org/apache/camel/quarkus/component/kafka/CamelKafkaRoutes.java
##########
@@ -46,5 +65,33 @@ public void configure() throws Exception {
.messageIdRepositoryRef("kafkaIdempotentRepository")
.to("mock:idempotent-results")
.end();
+
+ // Kafka consumer that use Manual commit and performs the manual commit
+ from(KAFKA_CONSUMER_MANUAL_COMMIT)
+ .routeId("foo")
+ .to(SEDA_FOO)
+ .process(e -> {
+ KafkaManualCommit manual = e.getIn().getHeader(KafkaConstants.MANUAL_COMMIT, KafkaManualCommit.class);
+ manual.commitSync();
+ });
+
+ // Kafka consumer that use Maunal commit and doesn't perform the manual commit
Review comment:
A small typo there ?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
zbendhiba commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714802971
##########
File path: integration-tests/kafka/src/test/java/org/apache/camel/quarkus/component/kafka/it/CamelKafkaTest.java
##########
@@ -86,4 +89,127 @@ void testQuarkusKafkaClientFactoryNotConfigured() {
.statusCode(200)
.body(is("true"));
}
+
+ @Test
Review comment:
And the idea is to verify at some point if it's committed, or never ? I'm trying to understand how to replace the last startup
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zbendhiba commented on a change in pull request #3124: Improve Kafka test coverage
Posted by GitBox <gi...@apache.org>.
zbendhiba commented on a change in pull request #3124:
URL: https://github.com/apache/camel-quarkus/pull/3124#discussion_r714775001
##########
File path: integration-tests/kafka/src/main/java/org/apache/camel/quarkus/component/kafka/CamelKafkaRoutes.java
##########
@@ -46,5 +65,33 @@ public void configure() throws Exception {
.messageIdRepositoryRef("kafkaIdempotentRepository")
.to("mock:idempotent-results")
.end();
+
+ // Kafka consumer that use Manual commit and performs the manual commit
+ from(KAFKA_CONSUMER_MANUAL_COMMIT)
+ .routeId("foo")
+ .to(SEDA_FOO)
+ .process(e -> {
+ KafkaManualCommit manual = e.getIn().getHeader(KafkaConstants.MANUAL_COMMIT, KafkaManualCommit.class);
+ manual.commitSync();
+ });
+
+ // Kafka consumer that use Maunal commit and doesn't perform the manual commit
Review comment:
oh yes !
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org