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

Provide support for disabling cron-based scheduled jobs registered via SchedulingConfigurer #23568

Closed
vpavic opened this issue Sep 2, 2019 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@vpavic
Copy link
Contributor

vpavic commented Sep 2, 2019

Support for special cron value of - (as defined in Scheduled#CRON_DISABLED) was introduced in #21397 however this is limited to use of @Scheduled.

For scheduled jobs registered via SchedulingConfigurer this is not supported which means there's no way to disable scheduled job by supplying the cron value.

This can be demonstrated using a simple Spring Boot based sample application:

@SpringBootApplication
@EnableScheduling
public class SampleApplication implements SchedulingConfigurer {

    private static final Logger logger = LoggerFactory.getLogger(SampleApplication.class);

    public static void main(String[] args) {
        SpringApplication.run(SampleApplication.class, args);
    }

    @Scheduled(cron = "${ticker1.cron:* * * * * ?}")
    public void ticker1() {
        logger.info("ticker1");
    }

    public void ticker2() {
        logger.info("ticker2");
    }

    @Value("${ticker2.cron:* * * * * ?}")
    private String ticker2Cron;

    @Override
    public void configureTasks(ScheduledTaskRegistrar taskRegistrar) {
        taskRegistrar.addCronTask(this::ticker2, this.ticker2Cron);
    }

}

This works equally well for all cases, except for - which will blow up with java.lang.IllegalArgumentException: Cron expression must consist of 6 fields (found 1 in "-") when supplied for ticker2.cron.

@sbrannen
Copy link
Member

This should be easy enough to support, and I'll take a look into it in detail.

public void configureTasks(ScheduledTaskRegistrar taskRegistrar) {
   taskRegistrar.addCronTask(this::ticker2, this.ticker2Cron);
}

In the interim, you can effectively replace the above with the following for a workaround.

public void configureTasks(ScheduledTaskRegistrar taskRegistrar) {
   if (!Scheduled.CRON_DISABLED.equals(this.ticker2Cron)) {
      taskRegistrar.addCronTask(this::ticker2, this.ticker2Cron);
   }
}

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 12, 2019
@sbrannen sbrannen self-assigned this Sep 12, 2019
@sbrannen sbrannen added this to the 5.2 GA milestone Sep 12, 2019
@sbrannen
Copy link
Member

This has been addressed for Spring Framework 5.2 in d689ef8.

@jhoeller and @snicoll, the CRON_DISABLED constant is now duplicated in ScheduledTaskRegistrar and @Scheduled. Ideally we should define that only once. So if you think we should make any changes regarding the above commit, please let me know.

@vpavic
Copy link
Contributor Author

vpavic commented Sep 12, 2019

Thanks for the quick resolution @sbrannen! 👍

sbrannen added a commit that referenced this issue Sep 16, 2019
This commit removes the duplicated CRON_DISABLED constant value from
@Scheuled and simply refers to the ScheduledTaskRegistrar.CRON_DISABLED
constant.

This avoids a potential package cycle by ensuring that the `annotation`
package depends on the `config` package but not the other way around.

See gh-23568
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants