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

add DoWithJobDetails passes a copy of the job to the jobFunc #307

Merged
merged 10 commits into from
Apr 26, 2022

Conversation

JohnRoesler
Copy link
Contributor

@JohnRoesler JohnRoesler commented Mar 5, 2022

What does this do?

  • adds a new method DoWithJobDetails that expects the passed jobFunc to have the it's final function parameter be a gocron.Job
    • an error is returned if the function doesn't meet this requirement
  • changes the Job mutex to be inside a pointer struct inside the Job to allow copying the job struct not by pointer
  • copy's the job parameters slice, since slices are by reference, to avoid a race condition with the executor calling the params
  • move the schedule next run logic before running the job so the job knows it's next run during the current run

Which issue(s) does this PR fix/relate to?

resolves #299

closes #251

List any changes that modify/break current functionality

Have you included tests for your changes?

👍

Did you document any new/modified functionality?

  • Updated example_test.go
  • Updated README.md

Notes

main test code

package main

import (
	"github.com/go-co-op/gocron"
	"log"
	"time"
)

func task(job gocron.Job) {
	log.Printf("job last run: %s, this run: %s, job next run: %s", job.LastRun(), time.Now(), job.NextRun())
}

func main() {
	s := gocron.NewScheduler(time.Local)
	_, err := s.Tag("tag1").Every("1s").DoWithJobDetails(task)
	if err != nil {
		log.Fatalf("something went wrong: %v", err)
	}
	s.StartBlocking()
}

Output

2022/03/05 23:41:27 job last run: 0001-01-01 00:00:00 +0000 UTC,        this run: 2022-03-05 23:41:27.058901 -0600 CST m=+0.000511084, job next run: 2022-03-05 23:41:28.058794 -0600 CST
2022/03/05 23:41:28 job last run: 2022-03-05 23:41:27.058795 -0600 CST, this run: 2022-03-05 23:41:28.059964 -0600 CST m=+1.001576834, job next run: 2022-03-05 23:41:29.059843 -0600 CST
2022/03/05 23:41:29 job last run: 2022-03-05 23:41:28.059869 -0600 CST, this run: 2022-03-05 23:41:29.061094 -0600 CST m=+2.002707918, job next run: 2022-03-05 23:41:30.060961 -0600 CST
2022/03/05 23:41:30 job last run: 2022-03-05 23:41:29.061044 -0600 CST, this run: 2022-03-05 23:41:30.062407 -0600 CST m=+3.004022834, job next run: 2022-03-05 23:41:31.062168 -0600 CST
2022/03/05 23:41:31 job last run: 2022-03-05 23:41:30.062301 -0600 CST, this run: 2022-03-05 23:41:31.063563 -0600 CST m=+4.005180626, job next run: 2022-03-05 23:41:32.06342 -0600 CST

@JohnRoesler JohnRoesler requested a review from Streppel March 5, 2022 22:04
@JohnRoesler
Copy link
Contributor Author

JohnRoesler commented Mar 5, 2022

This is missing a big piece right now. The next run isn't calculated until after the job is run. That's a key piece of information the caller might like to know. That will require some additional digging into.

Updated

@JohnRoesler JohnRoesler changed the title add DoWithDetails passes a copy of the job to the jobFunc add DoWithJobDetails passes a copy of the job to the jobFunc Mar 6, 2022
@JohnRoesler JohnRoesler merged commit a8dd701 into main Apr 26, 2022
@JohnRoesler JohnRoesler deleted the doWithDetails branch April 26, 2022 13:41
@defc0n
Copy link

defc0n commented Apr 27, 2022

Thanks for this!

@JohnRoesler
Copy link
Contributor Author

let me know how it works for you!

@defc0n
Copy link

defc0n commented May 7, 2022

let me know how it works for you!

Believe it is working for me and fixes my race condition when determining the next time the job will run, thanks. Took me a bit of time to realize the task function is passed as the first argument to DoWithJobDetails() but received as the last argument in that task function.

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

Successfully merging this pull request may close these issues.

[FEATURE] - Add new DoWithJob() function [BUG] - Makefile commands are out of date
3 participants