-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
1-31/10 vs */10 dom #70
Comments
I believe that is due to my reading of https://en.wikipedia.org/wiki/Cron:
|
Wouldn't that mean strictly I'm wondering whether starBit should be set in the case of |
Yeah, I agree |
@robfig when I remove starBit when step > 1 the following test fails:
Just want to make sure I'm not missing something here. Since Dom and Dow is an OR, it actually should be Changes made: diff --git a/parser.go b/parser.go
index 01c4c3a..40b824b 100644
--- a/parser.go
+++ b/parser.go
@@ -225,6 +225,10 @@ func getRange(expr string, r bounds) (uint64, error) {
if singleDigit {
end = r.max
}
+
+ if step > 1 {
+ extra_star = 0
+ }
default:
return 0, fmt.Errorf("Too many slashes: %s", expr)
} |
I imagine the test is wrong in that case |
Relevant portion of the spec (https://en.wikipedia.org/wiki/Cron): While normally the job is executed when the time/date specification fields all match the current time and date, there is one exception: if both "day of month" (field 3) and "day of week" (field 5) are restricted (not "*"), then one or both must match the current day.[3] When originally written, I incorrectly allowed restricted stars like "*/10" to trigger the "all must match" behavior. This bug would cause some schedules to run less frequently, or not at all. Fixes #70
This is fixed on v3 branch. |
Relevant portion of the spec (https://en.wikipedia.org/wiki/Cron): While normally the job is executed when the time/date specification fields all match the current time and date, there is one exception: if both "day of month" (field 3) and "day of week" (field 5) are restricted (not "*"), then one or both must match the current day.[3] When originally written, I incorrectly allowed restricted stars like "*/10" to trigger the "all must match" behavior. This bug would cause some schedules to run less frequently, or not at all. Fixes robfig#70
Fwiw, I think that the old behavior may have been correct here? The Wikipedia page looks to have been updated to match the man page: Wikipedia:
man page:
Meaning Came here after looking for a way to implement the behavior outlined in this blog post and wondering if this library implemented the AND or the OR. It's pretty esoteric behavior but I figured I'd point it out since I noticed. |
I think that we have to reference to the standard, instead of wikipedia. Below is the POSIX description:
It weird, but it the standard. Here is 10 years old discussion about this. |
@CompuRoot The POSIX standard does not appear to handle slashes at all:
|
@williambanfield |
I would think
1-31/10
vs*/10
would be the same, but the code handles this case differently:master/spec.go
Is there any doc somewhere that specifies why this is?
I'm asking because I added week year to my fork, and this is how I'm currently handling it:
wy-field/spec.go
I'm wondering if
domMatch && dowMatch && woyMatch
is correct in thestarBit
case.The text was updated successfully, but these errors were encountered: