Closed yob closed 1 year ago
Hello,
thanks for reporting that.
Double-checking for myself.
irb> `uname -a`
=> "OpenBSD somalia 7.1 GENERIC.MP#3 amd64\n"
irb> RUBY_VERSION
=> "2.7.6"
irb> Fugit::Cron.do_parse('30 14 * * 4%4+3 Australia/Melbourne').next_time(t).utc
=> 2022-10-06 03:30:00 UTC
irb> ENV['TZ'] = 'Australia/Melbourne'
=> "Australia/Melbourne"
irb> Fugit::Cron.do_parse('30 14 * * 4%4+3 Australia/Melbourne').next_time(t).to_t
=> 2022-10-06 14:30:00 +1100
irb> Fugit::Cron.do_parse('30 14 * * 4%4+3 Australia/Melbourne').next_time(t).utc
=> 2022-10-06 03:30:00 UTC
Timezone transition, according to https://www.timeanddate.com/time/zone/australia/melbourne is on Sunday, 2nd of October at 2am.
irb> Fugit.parse('2022-09-22 04:00:00 UTC').rweek
=> 195
irb> Fugit.parse('2022-09-22 04:00:00 UTC').rweek % 4
=> 3
irb> (Fugit.parse('2022-09-22 04:00:00 UTC').rweek + 3) % 4
=> 2
irb> Fugit.parse('2022-10-06 04:00:00 UTC').rweek % 4
=> 1
irb> (Fugit.parse('2022-10-06 04:00:00 UTC').rweek + 3) % 4
=> 0
OK, I need to get this calculation right. Stay tuned.
Meanwhile, wouldn't something like "30 14 * * thu#last Australia/Melbourne"
be easier to grok, although not strictly equivalent? https://github.com/floraison/fugit#the-hash-extension
Kind regards.
Thanks for taking a look.
Meanwhile, wouldn't something like "30 14 thu#last Australia/Melbourne" be easier to grok, although not strictly equivalent?
Yer, I assume that might work pretty well. In our case, for better or worse we're using fugit to parse cronlines for many thousands of customer controlled schedules so we don't have direct control over the format people choose.
The good news is only 0.4% of cronlines from our data set changed behaviour between 1.3.2
and 1.6.0
, and nearly all of them use X%4+Y
. They're %4
specifically too, cronlines with %1
, %2
and %3
don't appear in the list of cronlines that changed.
The other common input that changed is a trailing comma, like 5,15,25,35,45,55, * * * *
. These used to parse and now they raise an error, but we can clean these particular issue up in the dataset prior to upgrading so it's less challenging to handle.
def show(mod0, mod1)
puts "%#{mod0}+#{mod1}"
(mod0 + 1).times do |i|
print "%3d: " % [
i ]
print "%d %% %d --> %d" % [
i, mod0, i % mod0 ]
print " | %d %% %d == %d --> %5s" % [
i, mod0, i % mod0, (i % mod0) == mod1 ]
print " | (%d + %d) %% %d == 0 --> %5s" % [
i, mod0, mod1, (i + mod1) % mod0 == 0 ]
print " | %d %% %d == %d %% %d --> %5s" % [
i, mod0, mod1, mod0, i % mod0 == mod1 % mod0 ]
puts
end
end
puts; show(2, 1)
puts; show(3, 2)
puts; show(2, 2)
puts; show(2, 4)
puts; show(4, 3)
Gives
%2+1
0: 0 % 2 --> 0 | 0 % 2 == 0 --> false | (0 + 2) % 1 == 0 --> false | 0 % 2 == 1 % 2 --> false
1: 1 % 2 --> 1 | 1 % 2 == 1 --> true | (1 + 2) % 1 == 0 --> true | 1 % 2 == 1 % 2 --> true
2: 2 % 2 --> 0 | 2 % 2 == 0 --> false | (2 + 2) % 1 == 0 --> false | 2 % 2 == 1 % 2 --> false
%3+2
0: 0 % 3 --> 0 | 0 % 3 == 0 --> false | (0 + 3) % 2 == 0 --> false | 0 % 3 == 2 % 3 --> false
1: 1 % 3 --> 1 | 1 % 3 == 1 --> false | (1 + 3) % 2 == 0 --> true | 1 % 3 == 2 % 3 --> false
2: 2 % 3 --> 2 | 2 % 3 == 2 --> true | (2 + 3) % 2 == 0 --> false | 2 % 3 == 2 % 3 --> true
3: 3 % 3 --> 0 | 3 % 3 == 0 --> false | (3 + 3) % 2 == 0 --> false | 3 % 3 == 2 % 3 --> false
%2+2
0: 0 % 2 --> 0 | 0 % 2 == 0 --> false | (0 + 2) % 2 == 0 --> true | 0 % 2 == 2 % 2 --> true
1: 1 % 2 --> 1 | 1 % 2 == 1 --> false | (1 + 2) % 2 == 0 --> false | 1 % 2 == 2 % 2 --> false
2: 2 % 2 --> 0 | 2 % 2 == 0 --> false | (2 + 2) % 2 == 0 --> true | 2 % 2 == 2 % 2 --> true
%2+4
0: 0 % 2 --> 0 | 0 % 2 == 0 --> false | (0 + 2) % 4 == 0 --> true | 0 % 2 == 4 % 2 --> true
1: 1 % 2 --> 1 | 1 % 2 == 1 --> false | (1 + 2) % 4 == 0 --> false | 1 % 2 == 4 % 2 --> false
2: 2 % 2 --> 0 | 2 % 2 == 0 --> false | (2 + 2) % 4 == 0 --> true | 2 % 2 == 4 % 2 --> true
%4+3
0: 0 % 4 --> 0 | 0 % 4 == 0 --> false | (0 + 4) % 3 == 0 --> false | 0 % 4 == 3 % 4 --> false
1: 1 % 4 --> 1 | 1 % 4 == 1 --> false | (1 + 4) % 3 == 0 --> true | 1 % 4 == 3 % 4 --> false
2: 2 % 4 --> 2 | 2 % 4 == 2 --> false | (2 + 4) % 3 == 0 --> false | 2 % 4 == 3 % 4 --> false
3: 3 % 4 --> 3 | 3 % 4 == 3 --> true | (3 + 4) % 3 == 0 --> false | 3 % 4 == 3 % 4 --> true
4: 4 % 4 --> 0 | 4 % 4 == 0 --> false | (4 + 4) % 3 == 0 --> false | 4 % 4 == 3 % 4 --> false
gh-47 goes from
def weekday_modulo_match?(nt, mod)
nt.rweek % mod[0] == mod[1]
end
to
def weekday_modulo_match?(nt, mod)
(nt.rweek + mod[1]) % mod[0] == 0
end
I want the results for pre gh-47, but the safety of gh-47. I think I will go with
def weekday_modulo_match?(nt, mod)
nt.rweek % mod[0] == mod[1] % mod[0]
end
Which is demonstrated above by the exploration code.
@yob unless you see a show stopper, I'll close and release 1.7.0 soon. Thanks!
Brilliant, thanks for the fast turnaround ❤️
I re-ran the current HEAD of main against our dataset and we're down to only 9 cronlines (out of many thousands) that changed behavior between 1.3.2 and 1.7.0 (pre release). All 9 look like bug fixes or intentional behavior changes, and definitely aren't related to this issue. Good to release I think!
For completeness, here's the 9 that changed behaviour. All tests were done with TZ=UTC
, and input time of Time.utc(2022,9,12,12,1,0)
1 8,12,15 * * 1-5 America/New_York
1 4,12,20 * * * US/Pacific
0 0 3 * * Australia/Melbourne
0 0/4 * * *
0/30 * * * *
0/10 * * * 1-5 America/New_York
0 4/12 * * *
0 0/12 * * * -09:00
30 7/4 * * *
Hello,
thanks for the detailed report. I am sticking with the changes in gh-43, gh-53, and gh-56. In my opinion, they make fugit "righter".
I've just released fugit 1.7.0, see https://rubygems.org/gems/fugit/versions/1.7.0
If there is anything wrong, please tell me.
Thanks again!
In my opinion, they make fugit "righter".
I agree.
I've just released fugit 1.7.0
Thanks!
Hello again,
I had a second look at those cron strings linked to gh-56 via gh-79.
This will be the new behaviour:
0 0/4 * * *
=> 0 */4 * * *
0/30 * * * *
=> 0-59/30 * * * *
0/10 * * * 1-5 America/New_York
=> 0-59/10 * * * 1-5 America/New_York
0 4/12 * * *
=> 0 4-23/12 * * *
0 0/12 * * * -09:00
=> 0 0-23/12 * * * -09:00
30 7/4 * * *
=> 30 7-23/4 * * *
I will release fugit 1.7.1 with this change very soon.
Thanks again!
Issue description
We're working on upgrading our system from fugit 1.3.2 to 1.6.0. While doing so, we noticed the hash+modulo extension changed behaviour in the 1.4.5 release:
Interestingly, the only change in that release was #47, which was reported by a colleague of mine - @ticky. We're only now getting around to upgrading and bringing in all the latest fixes.
Error and error backtrace (if any)
None
Expected behaviour
Based on the change it sounds like the intent was to support an additional edge case, so I was surprised to see a behaviour change.
I'm still wrapping my head around this syntax, so maybe
4%4+3
doesn't make much sense and changing the behaviour was inevitable. That's fine if so, we'll plan a careful transition with the impacted users. I wanted to check first though.Context
Please replace the content of this section with the output of the following commands: