amitree / delayed_job_recurring

Extends delayed_job to support recurring jobs
MIT License
84 stars 33 forks source link

Undefined Method '>' for array #32

Open andreas-it-dev opened 5 years ago

andreas-it-dev commented 5 years ago

Hi,

thank you for your gem, i am excited to use it. however, there seems to be an issue when trying to schedule a job for multiple days..

my scheduled job is defined as

class UpdateInstrumentData
  include Delayed::RecurringJob
  run_every 1.week
  run_at ['tuesday 01:00', 'wednesday 01:00', 'thursday 01:00', 'friday 01:00']
  queue 'instruments'
  def perform
    instruments = Instrument.order(id: :asc).pluck(:symbol)
    InstrumentPrice.import_historical_data(instruments)
  end
end

and i get the error:

/delayed_job_recurring-0.3.8/lib/delayed/recurring_job.rb:113:in block in next_future_time': undefined method>' for # (NoMethodError)

it crashs here

def next_future_time(times)
    times.select{|time| time > Time.now}.min
end

maybe i made a mistake in the job definition? though in your docs you do something similar:

MyTask.schedule(run_every: 1.week, run_at: ['sunday 8:00am', 'wednesday 8:00am'])

any idea what the issue is?

thanks, Andreas

andreas-it-dev commented 5 years ago

fwiw, i have a similar job but only running once per week and it doesnt throw that error

class UpdateInstruments
  include Delayed::RecurringJob
  run_every 1.week
  run_at 'saturday 1:00am'
  queue 'instruments'
  def perform
    instruments = Instrument.where.not(instrument_type: 'Forex').order(id: :asc).pluck(:symbol)
    Instrument.update_stock_data(instruments, include_data: true)
    fx_symbols = Instrument.where(instrument_type: 'Forex').order(id: :asc).pluck(:symbol)
    InstrumentPrice.import_historical_data(fx_symbols)
  end
end
sjieg commented 4 years ago

Hello, I tried to fork and fix the issue, but I'm not understanding the difference between passing arguments into .schedule or inside the class with Delayed::RecurringJob included. I was able to reproduce the issue in RSpec, but I ran into different problems.

What I do understand is that run_at inside the class expects a splat argument. This comes from the definition: def run_at(*times). What is happening with your values is @awunder is that your array is being wrapped in another array. So instead of passing 4 time arguments, you're passing 1 array argument.

Maybe confusing, maybe not, but you can fix your scheduled job by removing the brackets around the run_at function:

class UpdateInstrumentData
  include Delayed::RecurringJob
  run_every 1.week
  run_at 'tuesday 01:00', 'wednesday 01:00', 'thursday 01:00', 'friday 01:00'
  queue 'instruments'
  def perform
    instruments = Instrument.order(id: :asc).pluck(:symbol)
    InstrumentPrice.import_historical_data(instruments)
  end
end

Hope this helps!