davsclaus commented on code in PR #23597:
URL: https://github.com/apache/camel/pull/23597#discussion_r3319964993


##########
parent/pom.xml:
##########
@@ -226,6 +226,8 @@
         <hibernate-validator-version>9.1.0.Final</hibernate-validator-version>
         <hibernate-version>6.3.2.Final</hibernate-version>
         <hsqldb-version>2.7.4</hsqldb-version>
+        <mariadb-version>3.5.3</mariadb-version>

Review Comment:
   MariaDB4j version `3.3.1` does not appear to exist on Maven Central — the 
latest released version of `ch.vorburger.mariaDB4j:mariaDB4j` is **3.2.0**. 
Could you verify the correct version? The CI passed but downstream consumers 
may not have this artifact available.



##########
parent/pom.xml:
##########
@@ -137,8 +137,8 @@
         <dapr-workflow-version>1.17.2</dapr-workflow-version>
         <datasonnet-mapper-version>3.0.1.3</datasonnet-mapper-version>
         <debezium-version>3.5.1.Final</debezium-version>
-        
<debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version>
-        <derby-version>10.16.1.1</derby-version>
+       
<debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version>

Review Comment:
   These two lines use tab indentation instead of 4 spaces, which is 
inconsistent with the rest of the file:
   ```suggestion
           
<debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version>
   ```



##########
components/camel-sql/pom.xml:
##########
@@ -48,7 +47,7 @@
         </dependency>
 
         <!-- test dependencies -->
-        <dependency>
+           <dependency>

Review Comment:
   Tab character used here — should be 8 spaces (2 levels of 4-space indent) to 
match the rest of the POM:
   ```suggestion
           <dependency>
   ```



##########
components/camel-sql/src/test/java/org/apache/camel/component/sql/stored/SqlFunctionDataSourceTest.java:
##########
@@ -16,64 +16,147 @@
  */
 package org.apache.camel.component.sql.stored;
 
+import java.sql.SQLException;
 import java.util.HashMap;
 import java.util.Map;
 
+import javax.sql.DataSource;
+
+import ch.vorburger.mariadb4j.DB;
+import ch.vorburger.mariadb4j.DBConfigurationBuilder;
 import org.apache.camel.Exchange;
 import org.apache.camel.builder.RouteBuilder;
 import org.apache.camel.component.mock.MockEndpoint;
+import org.apache.camel.component.sql.stored.template.TemplateParser;
 import org.apache.camel.test.junit6.CamelTestSupport;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
-import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
-import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.dao.DataAccessException;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.datasource.DriverManagerDataSource;
+import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator;
 

Review Comment:
   MariaDB4j ships native binaries only for linux64 (x86_64), macos-arm64, and 
win-x64. Camel CI also tests on `ppc64le` and `s390x`, where this test will 
fail because no MariaDB binary is available.
   
   Consider:
   - Adding `skipITs.ppc64le` and `skipITs.s390x` Maven properties in the POM, 
or
   - Using a JUnit `@DisabledIfSystemProperty` / architecture check to skip on 
unsupported platforms.



##########
parent/pom.xml:
##########
@@ -137,8 +137,8 @@
         <dapr-workflow-version>1.17.2</dapr-workflow-version>
         <datasonnet-mapper-version>3.0.1.3</datasonnet-mapper-version>
         <debezium-version>3.5.1.Final</debezium-version>
-        
<debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version>
-        <derby-version>10.16.1.1</derby-version>
+       
<debezium-mysql-connector-version>9.7.0</debezium-mysql-connector-version>
+       <derby-version>10.16.1.1</derby-version>

Review Comment:
   ```suggestion
           <derby-version>10.16.1.1</derby-version>
   ```



##########
components/camel-sql/pom.xml:
##########
@@ -70,17 +69,10 @@
             <version>${hamcrest-version}</version>
             <scope>test</scope>
         </dependency>
-        <!-- derby used for stored procedure tests -->
-        <dependency>
-            <groupId>org.apache.derby</groupId>
-            <artifactId>derby</artifactId>
-            <version>${derby-version}</version>
-            <scope>test</scope>
-        </dependency>
         <dependency>

Review Comment:
   Tab character used here:
   ```suggestion
               <version>${h2-version}</version>
   ```



##########
components/camel-sql/src/test/java/org/apache/camel/component/sql/stored/CallableStatementWrapperTest.java:
##########
@@ -35,17 +35,28 @@
 
 public class CallableStatementWrapperTest extends CamelTestSupport {
 
+    /* This is necessary when debugging tests to enable HSQLDB to find
+       the classes in the classpath. */
+    /*
+    static {
+        if (System.getProperty("hsqldb.method_class_names") == null) {
+            System.setProperty(
+                    "hsqldb.method_class_names",
+                    "org.apache.camel.component.sql.stored.*");
+        }
+    }
+    */
+
     private TemplateParser templateParser;
     private EmbeddedDatabase db;
     private JdbcTemplate jdbcTemplate;
     private CallableStatementWrapperFactory factory;

Review Comment:
   This commented-out `static {}` block is unnecessary since the 
`hsqldb.method_class_names` property is already set via the surefire plugin 
configuration in `pom.xml`. Please remove this dead code (also applies to 
`ProducerInOutTest` and `ProducerTest`).



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to