-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Adding fetch configuration option to hibernate-orm module #11950
Conversation
...ent/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java
Show resolved
Hide resolved
* @asciidoclet | ||
*/ | ||
@ConfigItem(defaultValue = "-1") | ||
public int batchFetchSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this make the property name .fetch.batch-fetch-size
? Can we eliminate the redundancy and call it .fetch.batch-size
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, new properties' names would be .fetch.batch-fetch-size
and .fetch.max-fetch-depth
. I am okay with removing the fetch
keyword as far as redundancy is concerned. Shall we also do the same for maxFetchDepth
so that it is .fetch.max-depth
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I mean for both of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkrzywanski can you adjust the names and squash everything?
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsmet done.
68a1dd2
to
6f0e2b7
Compare
...ent/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmConfigPersistenceUnit.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one looks good to go. I rebased and force pushed, let's wait for CI and merge.
Thanks!
Merged, thanks! |
This pull request should resolve #11527. New
fetch
configuration category has been added. If those properties are not set, fallback for older depracted properties is done (if configured).