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

[BUG] - CronJob withSeconds documentation and implementation inconsistency #738

Closed
rbroggi opened this issue Jun 20, 2024 · 0 comments · Fixed by #739
Closed

[BUG] - CronJob withSeconds documentation and implementation inconsistency #738

rbroggi opened this issue Jun 20, 2024 · 0 comments · Fixed by #739
Labels
bug Something isn't working

Comments

@rbroggi
Copy link
Contributor

rbroggi commented Jun 20, 2024

Describe the bug

The method CronJob documentation says that the 6th field is optional but it is actually mandatory (validation fails).

// CronJob defines a new job using the crontab syntax: `* * * * *`.
// An optional 6th field can be used at the beginning if withSeconds
// is set to true: `* * * * * *`.
// The timezone can be set on the Scheduler using WithLocation, or in the
// crontab in the form `TZ=America/Chicago * * * * *` or
// `CRON_TZ=America/Chicago * * * * *`
func CronJob(crontab string, withSeconds bool) JobDefinition {
	return cronJobDefinition{
		crontab:     crontab,
		withSeconds: withSeconds,
	}
}

The likely correction to this would be to change the following code block:

job.go:126

	if c.withSeconds {
		p := cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
		cronSchedule, err = p.Parse(withLocation)
	} else {
		cronSchedule, err = cron.ParseStandard(withLocation)
	}

with:

	if c.withSeconds {
		p := cron.NewParser(cron.SecondOptional | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
		cronSchedule, err = p.Parse(withLocation)
	} else {
		cronSchedule, err = cron.ParseStandard(withLocation)
	}

To Reproduce

Steps to reproduce the behavior:

Attempt to create a CronJob with the following expression * * * * * using the withSeconds parameter true.

Version

v2.5.0

(latest)

Expected behavior

When invoking CronJob with withSeconds set to true, we should accept both seconds and non-seconds cron-expressions (as the documentation claims).

Additional context

@rbroggi rbroggi added the bug Something isn't working label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant