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

Bad calculation #28

Open
jmlfr opened this issue Mar 2, 2016 · 4 comments
Open

Bad calculation #28

jmlfr opened this issue Mar 2, 2016 · 4 comments

Comments

@jmlfr
Copy link

jmlfr commented Mar 2, 2016

There is a bad calculation if I set more complex crontab. Here is an example.

2.2.4 :001 > require 'cron_parser'
=> true
2.2.4 :002 > require 'time'
=> true
2.2.4 :003 > cron_parser = CronParser.new('1 2 8-15 * 4')
=> #<CronParser:0x000000009644e8 @source="1 2 8-15 * 4", @time_source=Time>
2.2.4 :004 > cron_parser.next(Time.parse('2016-03-01 00:00'))
=> 2016-03-03 02:01:00 +0100

It should be '2016-03-10 02:01:00' I think.

@meesterdude
Copy link
Contributor

in the codebase:

# Careful, if both DOW and DOM fields are non-wildcard,
# then we only need to match *one* for cron to run the job

I would actually like to know why it does this - I get mixed results when i test the cron online between when trying to identify should be correct. What I do know is in the current operation, I can't define a rule for friday the 13th because it matches the next friday, OR the 13th; which I don't understand being desireable.

@siebertm can you shed some light on the reasonings?

@meesterdude
Copy link
Contributor

Actually, that appears to be the spec! that's just wild. So the logic is "the first, the 15th, or any friday" scheduling wise - I don't know why I'd want that. Or anyone. Meanwhile if it were AND, you could do things like "the first tuesday of the month".

I'll take a look at adding a flag to support the variation for those of us who want it to facilitate this kind of behavior while leaving the default behavior to meet the spec - and call out the subtly in the docs.

@meesterdude
Copy link
Contributor

see #29

@jmlfr
Copy link
Author

jmlfr commented Mar 21, 2016

Thanks a lot for patch, I'll try it ASAP ! :)

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