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

JobStart and JobEnd events not firing for every job when schedules are chained #96

Closed
chriswithpants opened this issue Aug 8, 2016 · 4 comments

Comments

@chriswithpants
Copy link

chriswithpants commented Aug 8, 2016

I'm logging when my jobs start and stop like this:

JobManager.JobStart += (info) => Console.WriteLine(info.Name + " starting");
JobManager.JobEnd += (info) => Console.WriteLine(info.Name + " completed");

When jobs are scheduled individually this works fine:

registry.Schedule<Job1>().ToRunNow().AndEvery(5).Seconds();
registry.Schedule<Job2>().ToRunNow().AndEvery(5).Seconds();
registry.Schedule<Job3>().ToRunNow().AndEvery(5).Seconds();

Output:

Job1 starting
Job2 starting
Job3 starting
Hello from Job2
Hello from Job1
Hello from Job3
Job2 completed
Job3 completed
Job1 completed

When jobs are scheduled in a chain, only the first job is reported.

registry.Schedule<Job1>()
        .AndThen<Job2>()
        .AndThen<Job3>().ToRunNow().AndEvery(5).Seconds();

Output:

Job1 starting
Hello from Job1
Hello from Job2
Hello from Job3
Job1 completed
@tallesl
Copy link
Contributor

tallesl commented Aug 12, 2016

I can reproduce alright, flagging it as a bug.

@tallesl tallesl added the bug label Aug 12, 2016
@chriswithpants
Copy link
Author

I had a quick look at fixing this but as the jobs are stored as actions we lose the ability to get a name out of them. Did you have any thoughts on how you wanted to tackle this?

@tallesl
Copy link
Contributor

tallesl commented Aug 19, 2016

I rushed to check if it was reproducible and to give the issue a label and ended up not realizing where the problem lies. My mistake.

Giving a second thought, this issue is more on the "it's not a bug, it's a feature" side of things. The real problem lies on the event names: it should be ScheduleStart, ScheduleEnd and ScheduleException instead of JobStart, JobEnd and JobException.

Even the JobManager name is troublesome now that I come to think about it (its AllSchedules and RunningSchedules properties shows what it's managing).

That being said, raising an "actual" job start and job end for each schedule's job shouldn't be hard to implement, the start event should be raised right before this block and the stop event right after.

The "ability to get a name out of them" is not a problem either. You have a name in the schedule event because the library stores a name for each schedule (if you don't assign one with .WithName("some name") it defaults to the name of the type of the first job). In this case the event handler should receive the order of the job that raised it (job 3 of 4 for instance), that's the ideal way to identify it considering that the schedule's jobs runs in a orderly fashion and they don't have a name.

I'm relabeling this issue as "feature change" to remind me of both renaming "Job" for "Schedule" where it's appropriate and to implement actual job start/end events (not schedule).

@tallesl
Copy link
Contributor

tallesl commented Sep 2, 2018

AndThen (chaining jobs) is about to be deprecated (#214).

@tallesl tallesl closed this as completed Sep 2, 2018
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

No branches or pull requests

2 participants