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

Introduce configuration to diable @Scheduled tasks #31101

Closed
wants to merge 1 commit into from

Conversation

Leewei222
Copy link

Prior to this commit, only cron @scheduled tasks can be disabled with the special cron expression value that indicates a disabled trigger: "-".

This commit enables this configuration to fixedDelay and fixedRate @scheduled tasks.

Fixes gh-28073

Prior to this commit, only cron @scheduled tasks can be disabled with the
special cron expression value that indicates a disabled trigger: "-".

This commit enables this configuration to fixedDelay and fixedRate @scheduled
tasks.

Fixes spring-projectsgh-28073
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 23, 2023
@snicoll
Copy link
Member

snicoll commented Aug 23, 2023

@Leewei222 can you share how this is fixing #28073? The issue there is rather a disable switch in testing scenario. Your proposal would require to change the underlying annotation. Also - for fixedDelay and fixedRate looks quite strange to me.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 23, 2023
@Leewei222
Copy link
Author

Hi @snicoll, thanks for the feedback.

The scheduled jobs are usually configured with property place holders. In some environments, we want to disable some of the schedulers. For cron scheduled job, this can easily be done by the following code

@Scheduled(cron = "${cron}")
 public void scheduleTaskWithCron() {
     // do something
}

// application.properties
cron=-

However, this cannot work for fixedDelay and fixedRate scheduled tasks. I think there should be an easy way for us to disable them too! My proposal can disable them with the following code

@Scheduled(fixedDelayString= "${fixedDelay}")
 public void scheduleTaskWithFixedDelay() {
     // do something
}

@Scheduled(fixedRateString= "${fixedRate}")
public void scheduleTaskWithFixedRate() {
     // do something
}

// application.properties
fixedDelay=-
fixedRate=-

If - for fixedDelay and fixedRate doesn't make sense, what about using negative number?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 23, 2023
@snicoll
Copy link
Member

snicoll commented Aug 23, 2023

OK that explains the use case. I don't think this actually fixes #28703, or at least not directly so let's discuss the merit of this one regardless.

If - for fixedDelay and fixedRate doesn't make sense, what about using negative number?

Both fixedDelay and fixedRate are actually numbers already with a convenience String-based attribute counter part. The default value for the long-based ones is already -1 to indicate that the value should not be taken into account. We can't unfortunately reuse this to signal something else.

If you want to disable some of the schedulers, I'd rather use a @Profile to prevent the scheduled bean to be exposed in the context at all. This should require some code refactoring but I believe this will for the better and more explicit than what you were suggesting.

Thanks for the PR, in any case!

@snicoll snicoll closed this Aug 23, 2023
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce NoOpTaskScheduler for disabling @Scheduled tasks in test setups
3 participants