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

support :tc: in connection URIs

Open gavinking opened this issue 3 years ago • 8 comments

@maxandersen has observed that the usual way to connect to test containers databases is to use a connection URI of form jdbc:tc:postgresql:9.6.8:///databasename.

Either we:

  1. convince @vietj that this is a feature that the Vert.x clients themselves should natively support, or
  2. emulate it in HR itself.

We actually already have the code to implement this, it's just in the test suite instead of the core where users can easily make use of it.

So what's it to be @vietj @aguibert? Does this go into Vert.x or does it stay here?

gavinking avatar Dec 03 '20 23:12 gavinking

https://www.testcontainers.org/modules/databases/jdbc/ and https://www.testcontainers.org/modules/databases/r2dbc/ for info where it is coming from.

maxandersen avatar Dec 03 '20 23:12 maxandersen

I don't think this would require any code changes in HR or Vertx, but instead a new module in Testcontainers itself. The R2DBC one works by registering a Testcontainers driver for R2DBC which just proxies to the actual driver. We would probably want to take a similar approach if we did this for Vertx drivers.

However, my 2 cents, I'm not a big fan of the :tc: URLs. I think it's too much magic going on and prefer to have containers explicitly defined/configured in the test code. So IMO wait to see if there is user demand for it (or an ambitious user could even contribute a vertx module to Testcontainers on their own)

aguibert avatar Dec 03 '20 23:12 aguibert

Well the issue is that the very first thing that Max tried to do with HR was to connect to postgres via a :tc: URI. And I can see him not being the only person to try that.

gavinking avatar Dec 03 '20 23:12 gavinking

(FTR, I'm not a great fan either.)

gavinking avatar Dec 03 '20 23:12 gavinking

The issue is that afaics the necessary code to wire it in is quite massive and redundant compared to this magic url.

Btw. It is not really different from how profiling jdbc drivers work. Those are also "magic" URLs.

maxandersen avatar Dec 04 '20 06:12 maxandersen

However, my 2 cents, I'm not a big fan of the :tc: URLs. I think it's too much magic going on and prefer to have containers explicitly defined/configured in the test code

Is it really that magical? In the end you are just specifying that you want to use tc in the url. It seems pretty straightforward for simple quick testing.

I don't think this would require any code changes in HR...

We might need to keep track of the :tc: when we parse the URL (I'm not sure, I haven't checked). But I agree that it doesn't seem like the driver should be part of the Hibernate Reactive project.

DavideD avatar Dec 04 '20 08:12 DavideD

The issue is that afaics the necessary code to wire it in is quite massive and redundant compared to this magic url.

Setting aside the issue of the "magic" URI, which FTR, doesn't have to be the way we expose this feature (we could do it via a separate property setting or whatever), it's true that we wrote a bunch of code for this, and it's code that is surely useful to at least some users.

So it seems like something we should share somewhere.

gavinking avatar Dec 04 '20 14:12 gavinking

The stuff we do with URL parsing is sort of a convenience trick so that people can re-use JDBC urls for non-JDBC stuff. Actually, a user should still be able to use a jdbc:tc: url and as long as the host/port/props match up it'll spin up a DB for you same as if we supported some sort of vertx:tc: protocol. Only downsides of this approach I can see are:

  1. Users would need a JDBC driver on their test classpath
  2. Testcontainers offers a convenience dbContainer.getConnection() method which would return a java.sql.Connection and the R2DBC driver offers an equivalent for their connection objects

IMO the best thing to do here is some variation of https://github.com/eclipse-vertx/vertx-sql-client/pull/644 which will give Vertx a proper service loader entry point rather than hard-coding, even if it's just an SPI.

aguibert avatar Dec 04 '20 16:12 aguibert