Skip to content

Rethink DefaultBatchConfiguration and/or change documentation #41958

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

Closed
micheljung opened this issue Aug 20, 2024 · 2 comments
Closed

Rethink DefaultBatchConfiguration and/or change documentation #41958

micheljung opened this issue Aug 20, 2024 · 2 comments
Labels
for: external-project For an external project and not something we can fix status: invalid An issue that we don't feel is valid

Comments

@micheljung
Copy link

micheljung commented Aug 20, 2024

Hi, I think DefaultBatchConfiguration has some issues that need to be addressed:

The documentation suggests, that it should be extended

Quote:

A typical usage of this class is as follows:

  @Configuration
  public class MyJobConfiguration extends DefaultBatchConfiguration {
 
      @Bean
      public Job job(JobRepository jobRepository) {
          return new JobBuilder("myJob", jobRepository)
                  // define job flow as needed
                  .build();
      }
 
  }
  1. This suggests to extend DefaultBatchConfiguration. But following Composition over Inheritance, you would rather want to import it.
  2. The only reason to extend DefaultBatchConfiguration should probably be to override e. g. getTaskExecutor(), allowing you to customize the Batch infrastructure. But then again, following Composition over Inheritance, it might be better to let the user provide a factory bean combined with a customizer.
  3. It uses factory beans like JobRepositoryFactoryBean, but these are hard coded and can neither be changed nor customized by the user.
  4. The code above suggests to define job beans in your subclass of DefaultBatchConfiguration. This mixes two things that, IMO, should not be mixed: Batch infrastructure configuration and job configurations. What if you want to have multiple classes that define jobs, would you then extend DefaultBatchConfiguration multiple times? If you do that, from which class will the beans be taken? Imagine overriding getTaskExecutor() in multiple subclasses. Only one will win. So as a user, you most likely want to subclass DefaultBatchConfiguration at most once and put jobs into separate configs. Therefore, the current doc might send users on the wrong path.

Beans can't be replaced

Even if customizable bean factories were provided, it would still not work well with Spring Boot AutoConfiguration where users expect beans to be replaceable. See #40040

Therefore, if you want to use a different JobRepository bean, you can't use this class and have to specify all the beans on your own.

I'm interested in reading your thoughts. When I find the time, I would also like to provide suggestions. Unfortunately, I can't right now.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 20, 2024
@wilkinsona
Copy link
Member

Thanks for the feedback but the structure of DefaultBatchConfiguration is out of Spring Boot's control as it's part of Spring Batch which is managed as a separate project.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2024
@wilkinsona wilkinsona added status: invalid An issue that we don't feel is valid for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 20, 2024
@micheljung
Copy link
Author

Argh, that's where I wanted to create the issue :) thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants