[Backgroundrb-devel] Schedule parser

Meng Kuan mengkuan at gmail.com
Mon Feb 4 03:13:37 EST 2008


On 01 Feb 2008, at 1:28 AM, hemant wrote:

>>  The above still did not solve the problem. So I dug deeper and found
>>  a possible logic bug in cron_trigger.rb file. It appears that
>>  cron_trigger incremented the day value beyond the number of days of
>>  the current month (from 31 to 32) and this caused the Time.local  
>> call
>>  to abort.
>>
>>  Here's my patch to adjust for this.
>>
>>  $ diff -c3 cron_trigger.rb cron_trigger.rb.new
>>  *** cron_trigger.rb     Thu Dec 20 17:27:47 2007
>>  --- cron_trigger.rb.new Thu Jan 31 14:25:57 2008
>>  ***************
>>  *** 126,131 ****
>>  --- 126,135 ----
>>              if next_hour < hour
>>                hour = next_hour
>>                day += 1
>>  +             if day > month_days(year, month)
>>  +               day -= month_days(year, month)
>>  +               month += 1
>>  +             end
>>                retry
>>              end
>>              hour = next_hour
>>
>>
>>  This seems to finally fix the problem for me.
>
> Thanks Meng, We will have this patch in.
>


Found another bug in cron_trigger. When I specify the hour as a range  
like "8-17", the parse_part method returns an empty array. This is  
because Range.new handles string arguments and integer arguments  
differently. Compare

   irb(main):001:0> Range.new("8","17")
   => "8".."17"
   irb(main):002:0> Range.new("8","17").to_a
   => []

to this:

   irb(main):003:0> Range.new(8, 17).to_a
   => [8, 9, 10, 11, 12, 13, 14, 15, 16, 17]


The fix is to convert the argument to integer first before handing it  
to Range.new. See the following patch.


$ diff -u cron_trigger.rb cron_trigger.rb.new
--- cron_trigger.rb     2007-12-20 17:27:47.000000000 +0800
+++ cron_trigger.rb.new 2008-02-04 16:04:53.000000000 +0800
@@ -215,7 +215,7 @@
        r = Array.new
        part.split(',').each do |p|
          if p =~ /-/  # 0-5
-          r << Range.new(*p.scan(/\d+/)).to_a.map do |x| x.to_i end
+          r << Range.new(*p.scan(/\d+/).map {|x| x.to_i}).to_a
          elsif p =~ /(\*|\d+)\/(\d+)/ and not range.nil?  # */5, 2/10
            min = $1 == '*' ? 0 : $1.to_i
            inc = $2.to_i


More information about the Backgroundrb-devel mailing list