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

Add JPA components only on single DataSource #6560

Closed

Conversation

kazuki43zoo
Copy link
Contributor

Update auto-configuration logic so that JPA components
are only added when there is a single candidate DataSource bean.
Related with gh-6449

Please review.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 4, 2016
Update auto-configuration logic so that JPA components
are only added when there is a single candidate DataSource bean.
Related with spring-projectsgh-6449
@kazuki43zoo kazuki43zoo force-pushed the multi-source-for-jpa branch from d29c458 to 3188f4f Compare August 4, 2016 15:39
@philwebb philwebb removed the status: waiting-for-triage An issue we've not yet triaged label Aug 10, 2016
@philwebb philwebb added this to the 1.4.1 milestone Aug 10, 2016
}
@Configuration
@ConditionalOnSingleCandidate(DataSource.class)
static class HibernateJpaConfiguration extends JpaBaseConfiguration {
Copy link
Member

Choose a reason for hiding this comment

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

That's a bold move but I think I like it. I have a feeling it's going to break user's app, wdyt @wilkinsona ?

Copy link
Member

Choose a reason for hiding this comment

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

I think it might too. I don't think such a change belongs in 1.4.1.

@philwebb philwebb removed this from the 1.4.1 milestone Aug 19, 2016
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 19, 2016
@snicoll snicoll added theme: datasource type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 10, 2017
@philwebb philwebb modified the milestone: 2.0.0.M4 Jan 12, 2017
@zachmarshall
Copy link

Coming in via #9394 so apologies if I've missed something, but should DataSourceAutoConfiguration also be included?

@snicoll
Copy link
Member

snicoll commented Jun 2, 2017

@zachmarshall if there is a DataSource (be it one or more) it's pointless to execute DataSourceAutoConfiguration. The PR needs to be reviewed but essentially the idea is to stop requiring a single DataSource and just backoff if there are more than one.

I really don't think that's a good way to fix your issue though. Let's keep the conversation on #9394 please.

@snicoll snicoll modified the milestones: 2.0.0.M3, 2.0.0.M4 Jun 2, 2017
@zachmarshall
Copy link

@snicoll what if you wanted DataSourceInitializer to run on the DataSource you create? Then DataSourceAutoConfiguration isn't useless right (since it creates the DataSourceInitializer)? Moreover, I'd really like to use a @Primary bean along with the autoconfigured DataSource that DataSourceAutoConfiguration creates (e.g. in the delegate pattern), but I understand if that is not supported and/or you think it shouldn't be done.

@snicoll
Copy link
Member

snicoll commented Jun 15, 2017

There is somewhat related issue for datasource initialization #9528

@snicoll snicoll modified the milestones: 2.0.0.M4, 2.0.0.M3, 2.0.0.M5 Jul 24, 2017
@snicoll
Copy link
Member

snicoll commented Oct 3, 2017

Duplicate of #5541

Thanks @kazuki43zoo for the PR, I ended up taking a slightly different route.

@snicoll snicoll closed this Oct 3, 2017
@snicoll snicoll added status: duplicate A duplicate of another issue and removed priority: normal type: enhancement A general enhancement labels Oct 3, 2017
@snicoll snicoll removed this from the 2.0.0.M5 milestone Oct 3, 2017
@kazuki43zoo kazuki43zoo deleted the multi-source-for-jpa branch November 24, 2017 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants