You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "ldrozdo (via GitHub)" <gi...@apache.org> on 2023/03/15 10:15:55 UTC
[GitHub] [camel-quarkus] ldrozdo opened a new pull request, #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
ldrozdo opened a new pull request, #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655
Fixes #4596
Covers all scenarios except for Cover different db types for jdbc , which was marked as optional.
--
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 #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#issuecomment-1482959966
Discussed in the community chat, indeed backporting this PR to 2.13.x is sufficient then.
--
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 diff in pull request #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1140453174
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/JdbcRoutes.java:
##########
@@ -44,5 +47,20 @@ public void process(Exchange exchange) throws Exception {
exchange.getIn().setBody(in);
}
});
+
+ from("direct://headers-as-parameters")
+ .to("jdbc:camel-ds?useHeadersAsParameters=true");
+
+ from("timer://interval-polling?delay=5000&period=1000")
Review Comment:
@ldrozdo Do you think the approach described above could add some 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.
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 diff in pull request #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1146623082
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/JdbcRoutes.java:
##########
@@ -44,5 +47,20 @@ public void process(Exchange exchange) throws Exception {
exchange.getIn().setBody(in);
}
});
+
+ from("direct://headers-as-parameters")
+ .to("jdbc:camel-ds?useHeadersAsParameters=true");
+
+ from("timer://interval-polling?delay=5000&period=1000")
Review Comment:
Yes, maybe there is a kind of transaction delay or alike so the record are not visible straight away.
I think it's ok to experiment with this, in case of flakiness, we could maybe wait in @PostConstruct that records are visible.
That's great job. It looks clearer (to me at least) :+1:
--
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 #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#issuecomment-1481671310
@ldrozdo Maybe the build issue could be solved by building with `-P format`. That should sort the imports.
--
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 diff in pull request #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1136910152
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/JdbcRoutes.java:
##########
@@ -44,5 +47,20 @@ public void process(Exchange exchange) throws Exception {
exchange.getIn().setBody(in);
}
});
+
+ from("direct://headers-as-parameters")
+ .to("jdbc:camel-ds?useHeadersAsParameters=true");
+
+ from("timer://interval-polling?delay=5000&period=1000")
Review Comment:
Maybe we could gain a bit of delay here.
For instance, could we consume records that would be added in `@PostConstruct` phase ?
And then, we could maybe use `timer` with `delay = 0` and `repeatCount = 1` ?
If this idea work, then maybe we could as well shorten `await()` in test ?
And maybe, it could also be possible to avoid producing a log by using a MockEnpoint like [here](https://github.com/apache/camel-quarkus/blob/main/integration-tests/netty/src/main/java/org/apache/camel/quarkus/component/netty/udp/NettyUdpResource.java#L88).
--
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] jamesnetherton commented on a diff in pull request #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1136889516
##########
integration-tests/jdbc/pom.xml:
##########
@@ -40,6 +40,14 @@
<groupId>org.apache.camel.quarkus</groupId>
<artifactId>camel-quarkus-log</artifactId>
</dependency>
+ <dependency>
Review Comment:
`camel-quarkus-bean-deployment` and `camel-quarkus-timer-deployment` dependencies need adding into the `virtualDependencies` profile.
##########
integration-tests/jdbc/src/test/java/org/apache/camel/quarkus/component/jdbc/CamelJdbcTest.java:
##########
@@ -114,8 +119,58 @@ void testHeadersFromSelectQuery() {
.and()
.body(not(containsString("CamelGeneratedColumns")))
.and()
- .body(containsString("CamelJdbcRowCount=2"))
+ .body(containsString("CamelJdbcRowCount"))
.and()
.body(containsString("CamelJdbcColumnNames=[ID, SPECIES]"));
}
+
+ @Test
+ void testNamedParameters() {
+ RestAssured.given()
+ .get("test/named-parameters/headers-as-parameters")
+ .then()
+ .body(containsString("{ID=1, SPECIES=Camelus dromedarius}"))
+ .and()
+ .body(containsString("{ID=2, SPECIES=Camelus bactrianus}"));
+ }
+
+ @Test
+ void testCamelJdbcParametersHeader() {
+ RestAssured.given()
+ .get("test/named-parameters/headers-as-parameters-map")
+ .then()
+ .body(containsString("{ID=2, SPECIES=Camelus bactrianus}"));
+ }
+
+ @Test
+ void testTimeIntervalDatabasePolling() {
+ RestAssured.given()
+ .contentType(ContentType.TEXT)
+ .body("insert into camelsGenerated (species) values ('Camelus status'), ('Camelus linus')")
+ .post("/test/execute");
+
+ await().atMost(20L, TimeUnit.SECONDS).pollDelay(10000, TimeUnit.MILLISECONDS).until(() -> {
Review Comment:
Can we speed this up and poll every 500 milliseconds (or less)?
--
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] ldrozdo commented on pull request #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "ldrozdo (via GitHub)" <gi...@apache.org>.
ldrozdo commented on PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#issuecomment-1482794018
@aldettinger thanks, sorry for that, it looks OK now.
--
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 merged pull request #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger merged PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655
--
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 #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#issuecomment-1482822081
Actually, that's a good question, I default to maintaining the last 2.x branch, maybe that could be discussed in chat.
--
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 diff in pull request #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1140453174
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/JdbcRoutes.java:
##########
@@ -44,5 +47,20 @@ public void process(Exchange exchange) throws Exception {
exchange.getIn().setBody(in);
}
});
+
+ from("direct://headers-as-parameters")
+ .to("jdbc:camel-ds?useHeadersAsParameters=true");
+
+ from("timer://interval-polling?delay=5000&period=1000")
Review Comment:
@ldrozdo Do you think this approach could add some 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.
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 diff in pull request #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1140451749
##########
integration-tests/jdbc/pom.xml:
##########
@@ -40,6 +40,14 @@
<groupId>org.apache.camel.quarkus</groupId>
<artifactId>camel-quarkus-log</artifactId>
</dependency>
+ <dependency>
Review Comment:
A simple `mvn process-resources -Pformat -N` from root directory should format this automatically.
--
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] ldrozdo commented on a diff in pull request #4655: Ref apache#4596: Expand JDBC tests - named parameters and samples
Posted by "ldrozdo (via GitHub)" <gi...@apache.org>.
ldrozdo commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1144414348
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/JdbcRoutes.java:
##########
@@ -44,5 +47,20 @@ public void process(Exchange exchange) throws Exception {
exchange.getIn().setBody(in);
}
});
+
+ from("direct://headers-as-parameters")
+ .to("jdbc:camel-ds?useHeadersAsParameters=true");
+
+ from("timer://interval-polling?delay=5000&period=1000")
Review Comment:
@aldettinger sorry for the late response, I had pto. Thank you for your hints, they sound good, I will try to implement them.
--
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 #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#issuecomment-1482801634
Well done @ldrozdo Do you think it's possible to backport to 2.13.x and 2.16.x branch ?
--
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] ldrozdo commented on a diff in pull request #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "ldrozdo (via GitHub)" <gi...@apache.org>.
ldrozdo commented on code in PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#discussion_r1146167370
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/JdbcRoutes.java:
##########
@@ -44,5 +47,20 @@ public void process(Exchange exchange) throws Exception {
exchange.getIn().setBody(in);
}
});
+
+ from("direct://headers-as-parameters")
+ .to("jdbc:camel-ds?useHeadersAsParameters=true");
+
+ from("timer://interval-polling?delay=5000&period=1000")
Review Comment:
@aldettinger I've implemented your hints, one thing I had to do differently is using `delay=2000` because with 0 it still sometimes failed.
--
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] jamesnetherton commented on pull request #4655: Ref #4596: Expand JDBC tests - named parameters and samples
Posted by "jamesnetherton (via GitHub)" <gi...@apache.org>.
jamesnetherton commented on PR #4655:
URL: https://github.com/apache/camel-quarkus/pull/4655#issuecomment-1482806518
> 2.16.x branch
Will there be any more releases off of that branch? I just closed https://github.com/apache/camel-quarkus/pull/4699 because I assumed there would not be.
--
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