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

Add getR2dbcUrl helper method to JdbcDatabaseContainer #8797

Open
mipo256 opened this issue Jun 23, 2024 · 10 comments
Open

Add getR2dbcUrl helper method to JdbcDatabaseContainer #8797

mipo256 opened this issue Jun 23, 2024 · 10 comments

Comments

@mipo256
Copy link

mipo256 commented Jun 23, 2024

Module

Core

Problem

I might've been chosen the wrong module, sorry for that, haven't found one for all RDBMS containers.

There is a convenient method, getJdbcUrl in JdbcDatabaseContainer. The problem however, if I'm working with R2DBC the connection string schema is different. Currently, I have to create an R2DBC connection string myself, which is a bit tedious.

Solution

It would be great to have a similar method in JdbcDatabaseContainer, like getR2dbcUrl that would construct the R2DBC compliant connection string.

Benefit

This would make integration with R2DBC drivers more seamless and provide better developer experience.

Alternatives

The alternative, I think, is to create the connection string manually, which might be fine, not a big deal, until you have to do it over and over again, in which case it became really frustrating.

Would you like to help contributing this feature?

Yes

@linghengqian
Copy link

  • I would say this sounds very magical and has little practical value. Although most databases will only have one R2DBC Driver, there may actually be multiple JDBC Drivers for a single database. For example, for the third-party PostgreSQL JDBC Driver https://github.com/awslabs/aws-postgresql-jdbc , there is obviously only jdbc:postgresql:aws://<host>:5432/<database>, but there is no corresponding r2dbc:postgresql:aws://<host>:5432/<database>.

@mipo256
Copy link
Author

mipo256 commented Jul 22, 2024

@linghengqian I do not understand your point at all. R2DBC is a spec. It is a spec. The format of such URL is predefined for each implementation. Therefore, it does not matter whether one RDBMS has 0, 1 or multiple R2DBC drivers - they still will comply to the standard.

I think adding this is utterly clear and portable, in contrast to example with JDBC URL which is not standardized by a single spec. If we care about the user, who is trying to get R2DBC URL for the database, which actually does not have the R2DBC driver - who's fault that actually is?

@eddumelendez
Copy link
Member

Hi, thanks for the suggestion. I don't think this is feasible nowadays, the change must be done in R2DBCDatabaseContainer. But, it would be used R2DBCDatabaseContainer.getR2dbcUrl(PostgreSQLR2DBCDatabaseContainer.getOptions(postgresContainer)), which I don't think is friendly enough.

Testcontainers R2dbc integrations in spring-boot and micronaut test-resources are based on PostgreSQLR2DBCDatabaseContainer.getOptions(postgresContainer)

Other alternative is to implement getR2dbcUrl in each implementation but given the spec I would rather prefer the having a general method that applies to all.

@linghengqian
Copy link

  • Given the ecological situation of r2dbc, I don't think r2dbc will continue to develop healthily, and testcontainers do not need to consider the interoperability of r2dbc and jdbc. An example is that https://central.sonatype.com/artifact/io.r2dbc/r2dbc-h2 , which is used to replace the h2database jdbc driver, has not had a new version for more than a year.

  • From the perspective of 2024, the ADBC ​​Driver involved in https://github.com/apache/arrow-adbc is more likely to become the real future of database clients, because ADBC ​​Driver is not designed just for Java, it also has specifications for a number of programming languages ​​such as C/Glib, C++, C#, Go, etc. for designing and implementing drivers. See https://arrow.apache.org/docs/format/ADBC.html .

@mipo256
Copy link
Author

mipo256 commented Oct 4, 2024

I honestly do not understand how the case for comparison of arrow-adbc and r2dbc specification can be made. These are meant for different things, that does not make any sense.

Apart from that, solving this issue does not seem to be that complicated (but I might be wrong). However, it is indeed a bit of shame that so much time spent for discussion and arguments for such a small feature.

@kiview
Copy link
Member

kiview commented Oct 7, 2024

The Testcontainers team has no negative sentiment towards R2DBC and no opinion with regards to its future direction. We aren't not opposed to the feature request from a user perspective at all, we just need to agree on the design/API.

For me it sounds like it could be possible to implement getR2dbcUrl() in JdbcDatabaseContainer and then have the individual container classes implement some schema specific getters. These are also totally fine to be opinionated (i.e. with regards to the driver), users can still opt to manually create the connection string instead and our getJdbcUrl() method is also highly opinionated.

Hi, thanks for the suggestion. I don't think this is feasible nowadays, the change must be done in R2DBCDatabaseContainer.

@eddumelendez Maybe you can elaborate why it would not be feasible with the current design?

@eddumelendez
Copy link
Member

I took another look and the following method can be provided

Example using MSSQLR2DBCDatabaseContainer

public static String getR2dbcUrl(MSSQLServerContainer<?> container) {
    ConnectionFactoryOptions options = getOptions(container);
    return String.format("...", options.getValue(Option.valueOf("")));
}

every R2DBCDatabaseContainer implementation must provide the static method.

@eddumelendez Maybe you can elaborate why it would not be feasible with the current design?

I would like to enforce the implementation by declaring the method in the interface but more breaking changes would be require. I think the suggestion above will cover the current need.

@kiview
Copy link
Member

kiview commented Oct 14, 2024

We could implement the method getR2dbcUrl() in the R2DBCDatabaseContainer interface with a default implementation that throws an UnsupportedOperationException, to avoid it being a breaking change to potential 3rd party implementations of the interface out there.

@mipo256 For your use case, is having this method on R2DBCDatabaseContainer appropriate, or would you want/need it on JdbcDatabaseContainer? I think also on JdbcDatabaseContainer, we could follow such an approach with a default UnsupportedOperationException implementation to avoid breaking changes.

@eddumelendez
Copy link
Member

We have agreed on adding a new static method on each R2DBCDatabaseContainer implementation

For example, under MySQLR2DBCDatabaseContainer

public static String getR2dbcUrl(MySQLContainer<?> container) {
    return ...;
}

Current users used to do MySQLR2DBCDatabaseContainer.getOptions(container), so, in order to make this discoverable we decided to proceed with static method.

Please free free to submit a PR.

@DHKIM-0511
Copy link

submit a PR #9569

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants