-
Notifications
You must be signed in to change notification settings - Fork 10
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
Allow "one-time" jobs #43
Comments
This is a good idea and enhancement! I see the value in supporting tasks that run once at the beginning of the lifecycle, such as migrations or data cleanup. However, these tasks are somewhat outside the primary purpose of the However, your suggestion brings up an interesting point about the flexibility of job scheduling. I agree that it could be beneficial to have a more generalized scheduling framework. This would allow for different types of scheduling mechanisms: Cron, One-Time, Periodic/Interval, and potentially others like iCalendar/RRule, although iCalendar might be beyond the current scope of this library. To incorporate this flexibility, one approach could be to develop a common abstraction or interface, say This design would keep the library modular and maintain SoC. |
Yes you are right - and while it might be easy to push that functionality inside |
Basically, with our abstraction, namely While the main theme of the library is (and will be) CRON, I don't see much speaking against introducing this concept. |
Yes it's very similar. If we want to collect these and run before any other job type however, we can add a property to JobDefinition that identifies this type of startup job. Then run something like this right before the call the ScheduleInitialJobs. private async Task ProcessStartupJobs(CancellationToken stopToken)
{
var startupJobs = registry.GetAllCronJobs().Where(j => j.IsStartupJob).ToList(); // or registry.GetAllOneTimeJobs
if (startupJobs.Any())
{
var startupTasks = new List<Task>();
foreach (var job in startupJobs)
{
startupTasks.Add(CreateExecutionTask(job, stopToken));
}
await Task.WhenAll(startupTasks);
}
} I don't think there's a need to juggle with signals to try to embed these into the regular loop. |
@linkdotnet for clarification, jobs we want to run at startup, like migrations, implies that they need to start and complete before anything else. The signature you designed assumes both a one time job that can run at startup, and a one time job that can run at any time. If so then I think the method name would be clearer as |
That is an interesting thought. I did think about them more simplistic. builder.Services.AddNCronJob(b => b.AddJob<TJob>().RunOnce()); Versus: builder.Services.AddNCronJob(b => b.AddJob<TJob>());
// ...
app.Services.GetRequiredService<IInstantJobRegistry>().RunInstantJob<TJob>(); In both cases I would expect that will apply. After all - there are "still jobs". I do think it would make sense to refine the requirement in the sense that we only allow |
Another question you raised (or answered) in your PR is if a "Startup-Job" should be awaited before any other "regular" CRON job or instant job gets executed. |
Allow the execution of jobs that are only run once at the beginning of the lifecycle. This could be some migration work or other tasks that should run very early on in the lifecycle.
Example
This will lead to a direct execution.
RunOnce
can have two overloads:To make that work, the
CronRegistry
has to be extended to distinguish between one-time jobs and CRON jobs.internal sealed partial class CronRegistry { + public IReadOnlyCollection<RegistryEntry> GetAllOneTimeJobs() => oneTimeJobs; }
This then can be picked up by the
CronSchedulder
to execute those once.The text was updated successfully, but these errors were encountered: