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

Implements fetcher runner by Cron instead of Config.MinFetchInterval #29

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

moisespsena
Copy link

Using default cron instance

import (
	"github.com/robfig/cron"
)

func main() {
	schedule, _ := cron.Parse("@daily")

	overseer.Run(overseer.Config{
		Program: prog,
		Address: ":3000",
		
		FetchCronSchedule: &schedule,
	})
}

Using your cron instance

import (
	"github.com/robfig/cron"
)

func main() {
	Cron := cron.New()
	Cron.Start()
	defer Cron.Stop()
	
	schedule, _ := cron.Parse("@daily")

	overseer.Run(overseer.Config{
		Program: prog,
		Address: ":3000",
		
		Cron: Cron,
		FetchCronSchedule: &schedule,
	})
}

@jpillora
Copy link
Owner

Thanks for the PR, however I intentionally left out the fetch interval from the main config, and left it to each fetcher to implement their own simple logic. For example the S3 fetcher, defaults to 5 minutes whereas the file fetcher defaults to 1 second, if the setting were in the main config (in the form of a duration or a cron schedule) then sane defaults would not be possible. I should probably make this clearer in the docs.

Maybe a cron helper might be useful:

func main() {
	overseer.Run(overseer.Config{
		Program: prog,
		Address: ":3000",
		Fetcher: &fetcher.HTTP{
			URL:      "http://localhost:4000/binaries/myapp",
			Interval: overseer.Cron("@daily"),
		},
	})
}

It would not need to return an error as this the user would not be writing cron strings, but rather it be hard-coded, or it be a selection of 4 interval options.

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.

2 participants