Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support :tc: in connection URIs #468

Open
gavinking opened this issue Dec 3, 2020 · 8 comments
Open

support :tc: in connection URIs #468

gavinking opened this issue Dec 3, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@gavinking
Copy link
Member

@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 gavinking added the enhancement New feature or request label Dec 3, 2020
@gavinking gavinking modified the milestone: FIRST CUT Dec 3, 2020
@maxandersen
Copy link
Member

@aguibert
Copy link
Contributor

aguibert commented Dec 3, 2020

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)

@gavinking
Copy link
Member Author

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
Copy link
Member Author

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

@maxandersen
Copy link
Member

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.

@DavideD
Copy link
Member

DavideD commented Dec 4, 2020

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.

@gavinking
Copy link
Member Author

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.

@aguibert
Copy link
Contributor

aguibert commented Dec 4, 2020

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 eclipse-vertx/vertx-sql-client#644 which will give Vertx a proper service loader entry point rather than hard-coding, even if it's just an SPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants