-
Notifications
You must be signed in to change notification settings - Fork 41.6k
Harmonize database initializers #9752
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
Conversation
snicoll
left a comment
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.
Thanks for the PR, I've added a few comments.
| * @author Vedran Pavic | ||
| * @since 2.0.0 | ||
| */ | ||
| public enum DatabaseInitializerMode { |
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.
I haven't looked at package tangle in details. Any reason why that's not in the jdbc package instead?
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.
While choosing the target package for DatabaseInitializerMode it seemed to me that it would be natural to place it next to AbstractDatabaseInitializer which is in org.springframework.boot.autoconfigure. I'm not sure what's the best place for those two.
|
|
||
| private final Initializer initializer = new Initializer(); | ||
| /** | ||
| * Create the required Spring Integration tables on startup. |
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.
That javadoc should change to imply that there is a "mode"
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.
OK, will address that.
|
|
||
| private final Initializer initializer = new Initializer(); | ||
| /** | ||
| * Create the required Spring Session tables on startup. |
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.
That removes silently a feature isn't it? It's probably for the better but I just want to double check.
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.
Hmm, not sure what exactly you refer to by removes silently a feature, but that part is basically alignment with what you did with Quartz JDBC properties.
This commit updates database initializers configuration to enable them automatically only when an embedded `DataSource` is used. Related configuration properties have been updated to use a more expressive `DatabaseInitializerMode` enum rather than `Boolean` flag.
|
Thanks for the feedback @snicoll - I've addressed the javadoc related suggestion. |
This commit updates database initializers configuration to enable them automatically only when an embedded `DataSource` is used. Related configuration properties have been updated to use a more expressive `DatabaseInitializerMode` enum rather than `Boolean` flag. See gh-9752
* pr/9752: Polish "Harmonize database initializers" Harmonize database initializers
This commit updates database initializers configuration to enable them automatically only when an embedded
DataSourceis used. Related configuration properties have been updated to use a more expressiveDatabaseInitializerModeenum rather thanBooleanflag.This resolves #8981.