search

vert-x3 / vertx-jdbc-client

JDBC support for Vert.x
Apache License 2.0
126 stars 90 forks source link

JDBCPool usage breaks on 4.3.0 #277

Open dustContributor opened 2 years ago

dustContributor commented 2 years ago

Hi! I'm using Vert.x 4.3.0. I asked about this issue on the Vert.x discord but it seems no dev saw it, so I'll report it here:

I was trying to use SQLite and since there doesn't seems to be a Vert.x client for it, I was using the generic jdbc client. 

// connStr has the SQLite connection. Simple pool initialization on the verticle start method like in the examples.
var pool = JDBCPool.pool(
  vertx, 
  new JDBCConnectOptions().setJdbcUrl(connStr), 
  new PoolOptions().setMaxSize(1));

When I try to use it with pool.query for instance, I get this:

java.lang.RuntimeException: java.lang.NoClassDefFoundError: io/agroal/api/configuration/supplier/AgroalDataSourceConfigurationSupplier
// tons of stuff then
Caused by: java.lang.NoClassDefFoundError: io/agroal/api/configuration/supplier/AgroalDataSourceConfigurationSupplier
    at io.vertx.jdbcclient.impl.AgroalCPDataSourceProvider.getDataSource(AgroalCPDataSourceProvider.java:66)

It seems that, while c3p0 dependency is mandatory, this JDBCPool object is actually trying to use Agroal, which is an optional dependency. The problem gets solved if I just add Agroal in my pom, but I'm wondering why it's set up this way if c3p0 isn't being used. And if Agroal is mandatory, why it's set as an optional dependency.

This may be related to #264

I do have other questions about this jdbc client but it seems no one is looking at the db channel in the Vert.x discord so I'm not sure where to ask.

Thank you for your time!

pmlopes commented 2 years ago

We should work on switching from c3p0 to agroal, as c3p0 seems to be abandoned. This would solve a couple of issue like this.

This will be mostly a big refactoring task as almost all tests default to c3p0 so we need a full review.

dustContributor commented 2 years ago

I see. It kinda sucks that just adding vertx-jdbc-client, creating a pool and running it fails and throws an exception if you do nothing else. I had to dig up a bit in the code to find out what was the workaround.

pmlopes commented 2 years ago

@dustContributor using the json configuration should default to c3p0, but indeed we should work on the flip.

If you want you could try to contribute this.

  1. update the pom.xml
  2. verify if test are still valid
  3. update config if needed to move away from c3p0
dustContributor commented 2 years ago

I'm assuming that making C3P0 dependency optional in the pom, while also making Agroal mandatory should be enough right?

I see some other stuff though, like the default pool class here: https://github.com/vert-x3/vertx-jdbc-client/blob/master/src/main/java/io/vertx/ext/jdbc/JDBCClient.java#L42

Or these docs https://github.com/vert-x3/vertx-jdbc-client/blob/master/src/main/asciidoc/index.adoc

Refering to C3P0 specific properties that probably someone experienced with Agroal should change.

The tests would run themselves if I make a PR right? Since you have them hooked to GitHub's CI