hibernate-reactive icon indicating copy to clipboard operation
hibernate-reactive copied to clipboard

automatic driver selection from URI

Open gavinking opened this issue 3 years ago • 13 comments

@aguibert did some work to automate the selection of the database driver when creating a Pool. Currently we still have a legacy workaround for 3.9.1. We need to clean that up.

UPDATE: Vert.x now has a way to do this. See: eclipse-vertx/vertx-sql-client#1101 To close this issue, at first glance, we need:

  • More tests related to how we extract the options from the URL (Relates to #1059)
  • Remove DefaultSqlClientPoolConfiguration.
  • Update or remove UriPoolConfiguration in favor of the new approach.

gavinking avatar Aug 27 '20 14:08 gavinking

https://github.com/eclipse-vertx/vertx-sql-client/issues/616

gavinking avatar Aug 27 '20 14:08 gavinking

Hrrrrm. @aguibert I was looking at this fragment of code in SqlClientPool:

		catch (ServiceConfigurationError e) {
			// Backup option if multiple drivers are on the classpath.
			// We will be able to remove this once Vertx 3.9.2 is available
			return findDriver( uri, e ).createPool( vertx, connectOptions, poolOptions );
		}

It seems to imply that we should now be able to delete the findDriver() method.

But actually it doesn't seem to be the case. On 3.9.2, it seems that the fallback stuff is still needed.

Can you clarify, please?

gavinking avatar Aug 27 '20 15:08 gavinking

Hrrrrm, I've just noticed that DB2ConnectOptions, MySQLConnectOptions, and PgConnectOptions all have a fromUri() method which parses a connection string.

I wonder if it would be better to use these connection strings than the JDBC URLs we've been using up to now.

It would be ideal if Vert.x offered a SqlConnectionOptions.forUri(String) method, but even without that we could iterate over the loaded Drivers and find one which accepts the given connection string.

WDYT @aguibert @DavideD @Sanne @vietj?

gavinking avatar Aug 27 '20 15:08 gavinking

I wonder if it would be better to use these connection strings than the JDBC URLs we've been using up to now.

I've actually demonstrated what that would look like, via the new SqlClientPoolConfiguration (which I added for a different reason).

https://github.com/hibernate/hibernate-reactive/blob/42937a41097a0e974c06d134f956488df7a4fecc/hibernate-reactive-core/src/test/java/org/hibernate/reactive/UriPoolConfiguration.java

gavinking avatar Aug 28 '20 09:08 gavinking

I need https://github.com/eclipse-vertx/vertx-sql-client/issues/767 to make progress on this.

gavinking avatar Sep 17 '20 16:09 gavinking

@gavinking Do you plan to work on this? Otherwise I think @blafond can start to have a first look

DavideD avatar Dec 14 '21 10:12 DavideD

Nah, I tried a while ago and I didn't have what I needed, but perhaps we do now with the new stuff in latest Vert.x.

gavinking avatar Dec 14 '21 10:12 gavinking

I mean to say no I'm not working on it :)

gavinking avatar Dec 14 '21 10:12 gavinking

The main problem with this issue is that in Hibernate Reactive we accept JDBC urls but Vert.x uses its own syntax depending on the database. So we would still need to parse the URL for most databases to convert it to something that works with Vert.x.

I'm wondering if we should accept a url with a different prefix for Vert.x. I think quarkus is using vertx-reactive:.

DavideD avatar Feb 25 '22 14:02 DavideD

I don't wanna make the call on that one. But perhaps you should ask around and see what others think.

I personally kinda like the ability to use JDBC URLs, but I'm not attached to them.

gavinking avatar Feb 26 '22 16:02 gavinking

I don't have a strong opinion. Personally I prefer configuration to be precise - as in explicit and unambiguous - but I know I'm in a minority.

So my personal preference would be to require the specific vertx format and offer no alternatives; especially as this sound like would never be fully complete.

Another reason for my preference is that I see libraries like Hibernate Reactive as libraries more than frameworks; let the integrating framework (Quarkus , vertx, any other..) deal with how configuration is preferrable to be exposed.

I honestly don't expect many users to bootstrap Hibernate Reactive (or ORM) in "Java SE" and dealing with our bootstrap process directly, so this implies any such convenience is both a)high maintenance and b) not facilitating much.

On the opposite spectrum; I know some people prefer to avoid frameworks and actually deal with our bootstrap directly - but these persons typically also prefer to avoid such magic conversions and I'd believe they would be fine to have to hardcode a specific driver choice and use a particular URI format.

Sanne avatar Feb 28 '22 09:02 Sanne

I don't think there's ambiguity about a URL that starts with jdbc: or vertx:.

Quarkus overrides already the creation of the pool so I don't think it will be affected. In Quarkus one can already create the connection using the prefix vertx-reactive:.

The main problem I see is that libraries like testcontainers return a JDBC url when the docker image is started. But it's the only example I can think of and I suppose one can set the configuration for the tests. Probably the port is the only value generated when the machine is started so it shouldn't be too hard to get it. Maybe it's time to have a look at the issue about starting testcontiners using a url prefix .

I guess I prefer the vertx format option because it means less work for us. This should only affect people using Hibernate Reactive on its own and only for some databases.

DavideD avatar Feb 28 '22 10:02 DavideD

I don't think there's ambiguity about a URL that starts with jdbc: or vertx:.

Many drivers offer jdbc connection properties which are declared on the jdbc URI. If we offer the ability to accept a JDBC uri people will assume they can paste any existing URL they have from a JDBC system they are migrating from, and will expect the same semantics to be honoured.

Except such properties are driver and vendor specific: vert.x will never be able to implement 1:1 same semantics for all those properties, so this facility is bound to provide an incomplete translation - hence a likely case of wrong expectations.

Sanne avatar Feb 28 '22 12:02 Sanne