Skip to content

Delegate detection of the database version to the Hibernate ORM dialect and enable checks for Hibernate Reactive #43764

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

Merged
merged 4 commits into from
Jun 12, 2025

Conversation

yrodiere
Copy link
Member

@yrodiere yrodiere commented Oct 8, 2024

Fixes #43703
Fixes #42255
Fixes #45263

Based on #41310, which must be merged first. Critically, #41310 requires Hibernate ORM 7.0.0.Final to be released, so we won't be able to merge this PR for a while. I'm submitting it anyway as a draft, so that we remember it's that easy: only the last two commits will be needed after we rebase. => #41310 was merged

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/hibernate-orm Hibernate ORM area/hibernate-reactive Hibernate Reactive area/jdbc Issues related to the JDBC extensions labels Oct 8, 2024
Copy link

quarkus-bot bot commented Oct 8, 2024

/cc @gsmet (hibernate-orm)

@yrodiere yrodiere removed area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/jdbc Issues related to the JDBC extensions labels Oct 8, 2024
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/jdbc Issues related to the JDBC extensions labels Oct 8, 2024
DavideD
DavideD previously requested changes Jan 8, 2025
Copy link
Contributor

@DavideD DavideD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging this, we should check that it works with Hibernate Reactive.

@yrodiere
Copy link
Member Author

I rebased on the ORM 7 upgrade PR (which upgrades to Reactive 3.0.0.CR1 too, now), and added tests to check DB version checking works with Hibernate Reactive, but so far it does not. I think QuarkusRuntimeInitDialectFactory is never used in reactive, leading to the DB version being unavailable and thus never checked.

I'll get back to this later, will try to fix it, and will update the Hibernate Reactive extension docs (since we need to document the DB version stuff there, too).

Copy link

github-actions bot commented May 12, 2025

🙈 The PR is closed and the preview is expired.

@yrodiere yrodiere changed the title Delegate detection of the database version to the Hibernate ORM dialect Delegate detection of the database version to the Hibernate ORM dialect and enable checks for Hibernate Reactive May 15, 2025
@yrodiere yrodiere force-pushed the i43703 branch 2 times, most recently from 7895d01 to 5ae0b5f Compare June 10, 2025 07:57
@yrodiere yrodiere marked this pull request as ready for review June 10, 2025 15:46
@yrodiere
Copy link
Member Author

Let's see what CI has to say, I'm seeing weird test failures for Gradle in my fork...

This comment has been minimized.

This comment has been minimized.

@yrodiere yrodiere marked this pull request as draft June 11, 2025 06:48
@yrodiere
Copy link
Member Author

There's definitely something wrong, and fortunately my new DbOfflineOnStartupTest seems to reproduce that. I'll need to have a closer look, but we'll miss the Quarkus 3.24.0.CR1 release. Well, we'll catch the next one :)

yrodiere added 4 commits June 11, 2025 14:15
Because some dialects have very specific ways of detecting the version,
such as running some SQL, that work better than asking the JDBC driver.
…licit Hibernate ORM dialects

Because that detection is now done by the Hibernate ORM dialect itself,
and should work properly for all dialects.
@yrodiere
Copy link
Member Author

There's definitely something wrong, and fortunately my new DbOfflineOnStartupTest seems to reproduce that. I'll need to have a closer look, but we'll miss the Quarkus 3.24.0.CR1 release. Well, we'll catch the next one :)

Should be good now.

@yrodiere yrodiere marked this pull request as ready for review June 11, 2025 16:48
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 11, 2025
Copy link

quarkus-bot bot commented Jun 11, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit b1e43b2.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingWorkingAppPropsTestCase.testContinuousTestingScenario3 - History

  • io.quarkus.builder.BuildException: Build failure: Build failed due to errors [error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor\#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7 at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:131) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733) at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856) - java.lang.RuntimeException
java.lang.RuntimeException: 
io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.redis.deployment.client.DevServicesRedisProcessor#startRedisContainers threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image docker.io/redis:7
	at io.quarkus.redis.deployment.client.DevServicesRedisProcessor.startRedisContainers(DevServicesRedisProcessor.java:131)
	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:733)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:856)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:255)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)

@yrodiere yrodiere merged commit 77d2e17 into quarkusio:main Jun 12, 2025
67 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.25 - main milestone Jun 12, 2025
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jun 12, 2025
@yrodiere
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/elasticsearch area/gradle Gradle area/hibernate-orm Hibernate ORM area/hibernate-reactive Hibernate Reactive area/hibernate-search Hibernate Search area/jdbc Issues related to the JDBC extensions area/maven area/panache area/spring Issues relating to the Spring integration kind/bugfix triage/flaky-test
Projects
None yet
3 participants