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

Initialize defaultCatalog and defaultSchema in Hibernate ORM config #23422

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Feb 3, 2022

No description provided.

@quarkus-bot quarkus-bot bot added area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE labels Feb 3, 2022
@gsmet gsmet requested a review from yrodiere February 3, 2022 19:19
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 3, 2022

Failing Jobs - Building 407910c

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/grpc-hibernate 

📦 integration-tests/grpc-hibernate

com.example.grpc.hibernate.BlockingRawTest.shouldAdd line 60 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in com.example.grpc.hibernate.BlockingRawTest that uses java.util.List was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Sure, go ahead.

However:

  1. If this is a problem, I'm surprised we don't have half of our testsuite failing with an NPE?
  2. Shouldn't quarkus-config do this for us? It seems like a sane default.

@gsmet
Copy link
Member Author

gsmet commented Feb 4, 2022

I know it's a problem because I had to do it for the others. Someone reported the NPE to me in private. I have no idea though why the test suite is not reporting the error.
The problem is not our config framework. We sometimes instantiate these objects ourselves when no config is defined (in the case of named persistence units).

@gsmet gsmet merged commit 3502f4f into quarkusio:main Feb 4, 2022
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Feb 4, 2022
@gsmet
Copy link
Member Author

gsmet commented Feb 4, 2022

My guess is that it's probably because we always have generation defined to drop/create the schema.

As long as we have one config in this root, it's not a problem.

@yrodiere
Copy link
Member

yrodiere commented Feb 4, 2022

The problem is not our config framework. We sometimes instantiate these objects ourselves when no config is defined (in the case of named persistence units).

Maybe we should not call the constructor directly in this case, but ask quarkus-config (whatever it's named, the module that deserializes config) to instantiate an empty config? That way, defaults are set correctly, and we don't have to be so careful about what we do in this file.

I'll look into this when I have some time.

@gsmet gsmet modified the milestones: 2.8 - main, 2.7.1.Final Feb 6, 2022
yrodiere added a commit to yrodiere/quarkus that referenced this pull request Aug 26, 2022
yrodiere added a commit to yrodiere/quarkus that referenced this pull request Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants