Skip to content

Conversation

@fmbenhassine
Copy link
Contributor

Before this commit, it was not possible to override the transaction manager by subclassing DefaultBatchConfigurer and overriding the getTransactionManager method.

This commit uses the getTransactionManager method in the initialize method in order to take into account the transaction manager provided by the user.

Resolves BATCH-2294


@Test
public void testConfigurationWithNoDataSourceAndNoTransactionManager() throws Exception {
AnnotationConfigApplicationContext applicationContext = new AnnotationConfigApplicationContext(BatchConfigurationWithNoDataSourceAndNoTransactionManager.class);
Copy link
Member

Choose a reason for hiding this comment

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

Nit pick...no reason to code to the implementation in these methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Fixed.

Before this commit, it was not possible to override the transaction
manager by subclassing DefaultBatchConfigurer and overriding the
getTransactionManager method.

This commit uses the getTransactionManager method in the initialize method
in order to take into account the transaction manager provided by the user.

Resolves BATCH-2294
@mminella
Copy link
Member

LGTM. Rebased and merged as a68d700.

@nightswimmings
Copy link

Isn't this solution problematic as long as SimpleBatchConfiguration keeps the

@Bean
	public PlatformTransactionManager transactionManager() throws Exception {

part?
Why not something like in the analogous SimpleTaskAutoConfiguration with a @ConditionalOnMissingBean?

@fmbenhassine
Copy link
Contributor Author

@ConditionalOnMissingBean is from Spring Boot. SCT can depend on boot, but we can't depend on boot in Spring Batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants