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

quarkus.datasource.db-version does not work with persistence.xml #43216

Closed
shawkins opened this issue Sep 11, 2024 · 9 comments
Closed

quarkus.datasource.db-version does not work with persistence.xml #43216

shawkins opened this issue Sep 11, 2024 · 9 comments
Labels
kind/bug Something isn't working

Comments

@shawkins
Copy link
Contributor

Describe the bug

Attempting to override the h2 version after the upgrade to Quarkus 3.14.2 results in an error mentioning that quarkus.datasource.db-version can be used. However when that is set, the error remains.

Expected behavior

quarkus.datasource.db-version should be honored.

Actual behavior

quarkus.datasource.db-version is not honored. I can see that the appropriate build time value is captured, but the check is always performed against

which results in an error.

How to Reproduce?

Try to downgrade h2 on quarkus 3.14.2 to a 2.2.x release.

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.14.2

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@shawkins
Copy link
Contributor Author

Tried to reproduce in just quarkus against 3.14.2 and the property worked as expected, so this is actually a keycloak issue.

@shawkins shawkins closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2024
@shawkins
Copy link
Contributor Author

shawkins commented Sep 11, 2024

@mabartos @vmuzikar this issue is being caused by having the default-persistence.xml - we hit

rather than which would propogate the setting as needed.

@yrodiere is there a workaround for this? Are values in the default-persistence.xml interpolated against the smallrye configuration such that we could add the mapping from quarkus.datasource.db-version to the jakarta property ourselves? - I believe the concensus from keycloak/keycloak#25384 was that env property references were not supported.

We also still have an active issue (keycloak/keycloak#27320) on moving away from using persistence.xml both internally and for customizations.

@yrodiere
Copy link
Member

this issue is being caused by having the default-persistence.xml

That's indeed a problem. From the docs:

If you use a persistence.xml, then you cannot use the quarkus.hibernate-orm.* properties and only persistence units defined in persistence.xml will be taken into account.


Are values in the default-persistence.xml interpolated against the smallrye configuration such that we could add the mapping from quarkus.datasource.db-version to the jakarta property ourselves? - I believe the concensus from keycloak/keycloak#25384 was that env property references were not supported.

They are not.

persistence.xml completely short-circuits the Quarkus configuration infrastructure. So you only get what plain Hibernate ORM would provide, which indeed does not include env property references.


@yrodiere is there a workaround for this?

Looking at the code, you can try to set the H2 version through configuration property jakarta.persistence.database-product-version in persistence.xml; I think this might just work, though I'm not entirely sure.

If you cannot hardcode it -- but for your investigation, surely you can? -- you can try setting it as a system property... Hibernate ORM might possibly pick it up, though I'm not certain.

IMPORTANT: This is only something you should do in the context of investigations. Overriding the version of H2 in Quarkus is completely untested and unsupported, because part of the H2 integration in Quarkus is tied to its internal code (in particular for native image). Changing the version of H2 may result in various unpredictable failures.
This is true for basically any dependency defined in the Quarkus BOM, by the way.


We also still have an active issue (keycloak/keycloak#27320) on moving away from using persistence.xml both internally and for customizations.

That would indeed spare you a lot of trouble. Beside these limitations, there are very few tests in Quarkus involving persistence.xml, so there are likely other problems we haven't (yet) encountered.

@vmuzikar
Copy link

I'd still argue that quarkus.datasource.db-version doesn't work with persistence.xml is a bug. The docs say that only quarkus.hibernate-orm.* options will be ignored.

@yrodiere
Copy link
Member

I'd still argue that quarkus.datasource.db-version doesn't work with persistence.xml is a bug. The docs say that only quarkus.hibernate-orm.* options will be ignored.

Okay, I'll update the docs. It should also state that Quarkus will not attempt to configure Hibernate ORM for you based on your environment, you need to put it all in persistence.xml.
For example, you already know you need to set the dialect yourself: Quarkus won't infer it from quarkus.datasource.db-kind. Similarly, you need to set the db version passed to the dialect yourself: Quarkus won't infer it from quarkus.datasource.db-version.

I suppose we could create an enhancement request (feel free to do so!) to make Quarkus configure datasource-related properties in Hibernate ORM, even when using persistence.xml. But:

  1. persistence.xml support is mainly here to help with migration, and I suspect arbitrarily overriding things that could already be configured in persistence.xml would actually get in the way of migrations.
  2. Such an enhancement, given its potential for breaking existing applications, is unlikely to get backported.

@yrodiere yrodiere changed the title quarkus.datasource.db-version does not work quarkus.datasource.db-version does not work with persistence.xml Sep 12, 2024
@yrodiere
Copy link
Member

Okay, I'll update the docs

Done here: #43240

I also tested that setting jakarta.persistence.database-product-version in persistence.xml will indeed work as expected. Though, as mentioned previously, this file does not support dynamic content (no ${myvar}, no references to environment variables). It's plain JPA, not Quarkus config.

@shawkins
Copy link
Contributor Author

Thanks for the follow up @yrodiere It looks like we won't need to use the workaround as h2 version 2.3.230 does not seem to exhibit the same problem.

@gsmet
Copy link
Member

gsmet commented Sep 12, 2024

@shawkins do you think we should downgrade H2 for 3.15.0?

@shawkins
Copy link
Contributor Author

@shawkins do you think we should downgrade H2 for 3.15.0?

Probably. I'll open an upstream issue in H2. They should be able to confirm what is happening with 2.3.232.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants