:first_at parameter is wonky

[concept47]

I run into this problem, over and over again when I'm trying to use scheduler.every with the :first_at parameter specified.
If I want to run a job every hour but specify that it should run first at 6am, then when my app starts up (lets say its 4pm) ... rufus goes and tries to run every job that SHOULD have run since 6am.

I guess this is the way it was designed, but I think it makes the :first_at parameter unusable.

For example, say I want to send out company wide emails every 2 weeks, but the start day was Jan 2nd 2011 or something like that ... I'd expect rufus to just schedule the next job, but instead it would go out and try to send out emails for all the weeks since the Jan 2nd 2011 ... which is really bad for me.

Is this something you'd be open to changing?
I'd be willing to work on it if you'd take a look at my code.

[jmettraux]

OK, I will fix that.

Thanks for reporting the issue.

[concept47]

Ah ... looks like specifying
:discard_past => true
is supposed to fix the problem ... but it doesn't ... the scheduler just won't run at all. bummer.

[concept47]

Okay ... it looks like this code works if you specify :discard_past => true
in lib/rufus/sc/jobs.rb

def determine_at
  return unless @frequency

  @last = @at
    # the first time, @last will be nil

  @at = if @last
    @last + @frequency
  else
    if fi = @params[:first_in]
      Time.now.to_f + Rufus.duration_to_f(fi)
    elsif fa = @params[:first_at]
        if @params[:discard_past]
            now = Time.now.to_f
            next_occurrence = fa.to_f
            while next_occurrence < now do
                next_occurrence += @frequency
            end
            next_occurrence
        else
            Rufus.at_to_f(fa)
        end
    else
      Time.now.to_f + @frequency
    end
  end
end

I'll use it till you update the official gem ... I hope its of some use to you.

[jmettraux]

OK, I will at first write a spec for the issue (and document :discard_past).

Thanks again,

[jmettraux]

Hello,

I don't think sliding :first_at is a good thing. Scheduler#every should start immediately if the first_at is in the past.

The behaviour you're trying to get is better obtained with scheduler#cron (every day at 6am for instance).

[concept47]

I don't think sliding :first_at is a good thing. Scheduler#every should start immediately if the first_at is in the past.

Why?

[jmettraux]

There are two options :

• raise an ArgumentError ":first_at is in the past"
• schedule every x starting now

The option you proposed is covered by scheduler#cron IMO.

The second option feels natural (at least for me).

[jmettraux]

Oh well, even if it's not natural and awkward to document, let's do it.

Taking your patch in (warning : fa.to_f, won't work well when fa is a string)

[concept47]

There are two options :
raise an ArgumentError ":first_at is in the past"
schedule every x starting now
The option you proposed is covered by scheduler#cron IMO.
The second option feels natural (at least for me).

Interesting way of looking at it.
I think that there is a scenario of using :first_at where it is after the server has started (you want to run and event x amount of times after the server starts up), but there is also a situation where it could be in the past and you're just using it as a timing device so that stuff happens (for example) every hour at the top of the hour (4pm, 5pm, 6pm etc) too and I think thats a valid use case.

I don't think that an error message is necessary because it seems to signal that there is something wrong with using it that way.

To your second point ... cron works for this simple case, but what about a scenario where you have something that is harder to write in cron ... like the case I had, where I wanted something to run every 2nd monday starting from a particular date (say April 18th 2011). That's actually non trivial to do via cron. but dead simple to do with this (its the main reason I actually started looking into this)

Now ... I'm not sure I get what you mean by "schedule every x starting now", but if it means schedule the next job that would have been scheduled from the present time ... then thats what my code is supposed to do ... if you mean "run the job then schedule the next one" then I don't think thats such a good option.

[concept47]

Oh well, even if it's not natural and awkward to document, let's do it.

:) Its your gem sir, but I really think you'll find people don't think its awkward at all

Taking your patch in (warning : fa.to_f, won't work well when fa is a string)

Awesome ... maybe we can make Chronic a dependency then run everything through it?
Or check for its (fa) class and throw an error asking them to use Chronic to do the transformation?

[jmettraux]

Many thanks.

Now, there will be someone complaining about that change, sooner or later. There are always people complaining it does not work as they think it should. Like you did ;-)

Best regards.

[concept47]

Nice!
Is there a way to install the gem from the github source or do I have to wait till you put out the next release.

[jmettraux]

Well, there is http://gembundler.com/git.html but I will release immediately then.

[jmettraux]

2.0.9 is out, thanks again !