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/04/06 13:12:43 UTC
[GitHub] [camel-quarkus] ldrozdo opened a new pull request, #4751: Ref#4596 Cover different db types for jdbc
ldrozdo opened a new pull request, #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751
Fixes #4596
Last part of expanding JDBC integration tests: covering different db types.
I've disabled some tests because of:
1. #4750 for postgresql
2. [For derby](https://github.com/quarkusio/quarkus/issues/23083)
I didn't use quarkus-flyway yet, as we discussed in #4730, it can be covered in next PRs.
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#discussion_r1159879595
##########
integration-tests/jdbc/src/main/resources/application.properties:
##########
@@ -18,6 +18,11 @@
#
# Quarkus :: DS
#
-quarkus.datasource.camel-ds.jdbc.url=jdbc:h2:tcp://localhost/mem:test
-quarkus.datasource.camel-ds.db-kind=h2
-quarkus.datasource.camel-ds.jdbc.max-size=8
+#quarkus.datasource.camel-ds.jdbc.url=jdbc:h2:tcp://localhost/mem:test
+#quarkus.datasource.camel-ds.db-kind=h2
+#quarkus.datasource.camel-ds.jdbc.max-size=8
Review Comment:
If this is like default value, could we remove the commented lines completely ?
--
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] zhfeng commented on pull request #4751: Ref#4596 Cover different db types for jdbc
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1499053480
It seems that we can introduce a [group testing](https://camel.apache.org/camel-quarkus/3.0.x/contributor-guide/extension-testing.html#_grouping) for all the supported db.
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1506904308
> @aldettinger I'm sorry, the checks are failing and I can't figure out why. Everything works on my side. Can you please have a look, maybe you will spot the issue? Thank you very much.
@ldrozdo The previous failure was just a transient connect failure we have more often those days. I think @essobedo was really nice to relaunch the job :+1: A bit of fingers crossed and it should pass :)
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1499167358
Well done @ldrozdo . That's good addition. There are few questions in the review.
> It seems that we can introduce a [group testing](https://camel.apache.org/camel-quarkus/3.0.x/contributor-guide/extension-testing.html#_grouping) for all the supported db.
Interesting idea @zhfeng , maybe it could be another ticket ?
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#discussion_r1159875497
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/CamelResource.java:
##########
@@ -88,22 +87,22 @@ public String getSpeciesById(@PathParam("id") String id) throws Exception {
@Produces(MediaType.TEXT_PLAIN)
public String getSpeciesByIdWithSelectList(@PathParam("id") String id) throws Exception {
List<LinkedHashMap<String, Object>> result = template
- .requestBody("jdbc:camel-ds?outputType=SelectList", "select * from camels where id = " + id, List.class);
+ .requestBody("jdbc:cameldb?outputType=SelectList", "select * from camels where id = " + id, List.class);
if (result.isEmpty()) {
throw new IllegalStateException("Expected at least 1 camel result but none were found");
}
LinkedHashMap<String, Object> data = result.get(0);
- return data.get("SPECIES") + " " + data.get("ID");
+ return data.toString();
Review Comment:
Is it really better to use toString() ? What happen if the toString() implementation changes ?
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#discussion_r1159878237
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/CamelResource.java:
##########
@@ -183,4 +183,47 @@ public String moveBetweenDatasources() throws Exception {
return template.requestBody("direct://move-between-datasources", null, String.class);
}
+ private void runScripts(String fileName) throws SQLException, IOException {
+ try (Connection conn = dataSource.getConnection()) {
+ try (Statement statement = conn.createStatement()) {
+ try (InputStream is = Thread.currentThread().getContextClassLoader()
+ .getResourceAsStream("sql/" + fileName);
+ InputStreamReader isr = new InputStreamReader(is);
+ BufferedReader reader = new BufferedReader(isr)) {
+
+ reader.lines().filter(s -> s != null && !"".equals(s) && !s.startsWith("--")).forEach(s -> {
+ try {
+ statement.execute(s);
Review Comment:
Ok, it's executing each line as a separate statement.
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "ldrozdo (via GitHub)" <gi...@apache.org>.
ldrozdo commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1499131528
#4750 was resolved, I've fixed it in the tests.
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger merged PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#discussion_r1159879595
##########
integration-tests/jdbc/src/main/resources/application.properties:
##########
@@ -18,6 +18,11 @@
#
# Quarkus :: DS
#
-quarkus.datasource.camel-ds.jdbc.url=jdbc:h2:tcp://localhost/mem:test
-quarkus.datasource.camel-ds.db-kind=h2
-quarkus.datasource.camel-ds.jdbc.max-size=8
+#quarkus.datasource.camel-ds.jdbc.url=jdbc:h2:tcp://localhost/mem:test
+#quarkus.datasource.camel-ds.db-kind=h2
+#quarkus.datasource.camel-ds.jdbc.max-size=8
Review Comment:
Could we remove the commented lines completely ?
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "ldrozdo (via GitHub)" <gi...@apache.org>.
ldrozdo commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1506727881
Thanks @aldettinger for your comments, I implemented them. Should I create issue for group testing which @zhfeng suggested?
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "ldrozdo (via GitHub)" <gi...@apache.org>.
ldrozdo commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1506830644
@aldettinger I'm sorry, the checks are failing and I can't figure out why. Everything works on my side. Can you please have a look, maybe you will spot the issue? Thank you very much.
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#discussion_r1159883962
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/CamelResource.java:
##########
@@ -183,4 +183,47 @@ public String moveBetweenDatasources() throws Exception {
return template.requestBody("direct://move-between-datasources", null, String.class);
}
+ private void runScripts(String fileName) throws SQLException, IOException {
+ try (Connection conn = dataSource.getConnection()) {
+ try (Statement statement = conn.createStatement()) {
+ try (InputStream is = Thread.currentThread().getContextClassLoader()
+ .getResourceAsStream("sql/" + fileName);
+ InputStreamReader isr = new InputStreamReader(is);
+ BufferedReader reader = new BufferedReader(isr)) {
+
+ reader.lines().filter(s -> s != null && !"".equals(s) && !s.startsWith("--")).forEach(s -> {
+ try {
+ statement.execute(s);
Review Comment:
Then, maybe we could put a comment ?
--
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 #4751: Ref#4596 Cover different db types for jdbc
Posted by "aldettinger (via GitHub)" <gi...@apache.org>.
aldettinger commented on code in PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#discussion_r1159871774
##########
integration-tests/jdbc/src/main/java/org/apache/camel/quarkus/component/jdbc/CamelResource.java:
##########
@@ -53,33 +62,23 @@ public class CamelResource {
@Inject
CamelContext context;
+ @ConfigProperty(name = "quarkus.datasource.cameldb.db-kind")
+ String dbKind;
+
@PostConstruct
void postConstruct() throws Exception {
- try (Connection con = dataSource.getConnection()) {
- try (Statement statement = con.createStatement()) {
- try {
- statement.execute("drop table camels");
- statement.execute("drop table camelsGenerated");
- } catch (Exception ignored) {
- }
- statement.execute("create table camels (id int primary key, species varchar(255))");
- statement.execute("create table camelsGenerated (id int primary key auto_increment, species varchar(255))");
- statement.execute("create table camelsProcessed (id int primary key auto_increment, species varchar(255))");
- statement.execute("insert into camelsGenerated (species) values ('Camelus status'), ('Camelus linus')");
- statement.execute("insert into camels (id, species) values (1, 'Camelus dromedarius')");
- statement.execute("insert into camels (id, species) values (2, 'Camelus bactrianus')");
- statement.execute("insert into camels (id, species) values (3, 'Camelus ferus')");
-
- context.getRouteController().startRoute("jdbc-poll");
- }
- }
+ runScripts("droptables.sql");
Review Comment:
Is it useful to open a connection on each runScripts call ? Maybe we could pass the connection as argument to runScript ?
--
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] zhfeng commented on pull request #4751: Ref#4596 Cover different db types for jdbc
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #4751:
URL: https://github.com/apache/camel-quarkus/pull/4751#issuecomment-1506743878
@ldrozdo yeah, please create a follow up issue for group testing. Thanks!
--
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